Skip to content

Conversation

@frankier
Copy link

@frankier frankier commented Mar 6, 2025

Continuing from davidanthoff/Electron.jl#140

This PR adds a package extension for URIs.jl. The implementation is taken from FilePaths.jl.

I think the version needs to be bumped so FilePaths.jl does not pull this in.

@rofinn
Copy link
Owner

rofinn commented Mar 7, 2025

It looks like the tests are failing because the URI version is causing the tests to use an old version of BSON (v0.2.6 vs v0.3.9). I'll need to look at adjusting the dependency versions to fix this. Otherwise, I don't see an issue.

Would you mind reminding me why it makes sense to include the extension here of FilePaths.jl? I know you opened an issue but didn't see any PRs. @davidanthoff I think you suggested having the extension here?

@rofinn
Copy link
Owner

rofinn commented Mar 7, 2025

Oh, I think I figured out what is happening. We're using JLSO.jl just as a test case, but JLSO.jl actively depends on FilePathsBase.jl. Bumping the FilePathsBase.jl release breaks the existing compatibility.

@davidanthoff
Copy link
Contributor

Hm, I have to admit I don't even know whether the "recommended" approach would be to have it here or in URIs.jl?

Between here and FilePaths.jl, I also don't really know. I think if we were to start today, we wouldn't have FilePathsBase.jl at all, right? Maybe the right strategy would actually be to move everything over into FilePaths.jl and give up on FilePathsBase.jl?

I think for the short-term fix it probably doesn't matter?

@rofinn
Copy link
Owner

rofinn commented Mar 7, 2025

Agreed. There would have been no need for FilePathsBase.jl if we had the 1.9 package extensions in the 0.5 days. In any case, I've made a PR on FilePaths.jl, hoping to address the immediate blocker.

rofinn/FilePaths.jl#65

CI jobs were disabled, but test pass locally and I've re-enabled the jobs for prior to tagging.

(FilePaths) pkg> test
     Testing FilePaths
      Status `/private/var/folders/sy/srj5df5j7jvf0_hb225496sh0000gn/T/jl_qLOJSi/Project.toml`
  [8fc22ac5] FilePaths v0.9.0 `~/repos/FilePaths.jl`
  [48062228] FilePathsBase v0.9.21
  [c27321d9] Glob v1.3.1
  [1914dd2f] MacroTools v0.5.13
  [189a3867] Reexport v1.2.2
  [30578b45] URIParser v0.4.1
  [5c2747f8] URIs v1.5.1
  [8dfed614] Test v1.11.0
      Status `/private/var/folders/sy/srj5df5j7jvf0_hb225496sh0000gn/T/jl_qLOJSi/Manifest.toml`
  [34da2185] Compat v4.14.0
  [8fc22ac5] FilePaths v0.9.0 `~/repos/FilePaths.jl`
  [48062228] FilePathsBase v0.9.21
  [c27321d9] Glob v1.3.1
  [1914dd2f] MacroTools v0.5.13
  [189a3867] Reexport v1.2.2
  [30578b45] URIParser v0.4.1
  [5c2747f8] URIs v1.5.1
  [2a0f44e3] Base64 v1.11.0
  [ade2ca70] Dates v1.11.0
  [b77e0a4c] InteractiveUtils v1.11.0
  [56ddb016] Logging v1.11.0
  [d6f4376e] Markdown v1.11.0
  [a63ad114] Mmap v1.11.0
  [de0858da] Printf v1.11.0
  [9a3f8284] Random v1.11.0
  [ea8e919c] SHA v0.7.0
  [9e88b42a] Serialization v1.11.0
  [fa267f1f] TOML v1.0.3
  [8dfed614] Test v1.11.0
  [cf7118a7] UUIDs v1.11.0
  [4ec0a83e] Unicode v1.11.0
Precompiling project for configuration --code-coverage=none --color=yes --check-bounds=yes --warn-overwrite=yes --depwarn=yes --inline=yes --startup-file=no --track-allocation=none...
  3 dependencies successfully precompiled in 1 seconds. 20 already precompiled.
     Testing Running tests...
┌ Warning: `URIParser` is deprecated, use `URIs` instead.
│   caller = ip:0x0
└ @ Core :-1
Test Summary: | Pass  Total  Time
FilePaths     |   36     36  3.5s
     Testing FilePaths tests passed

I'll make a separate issue to address the weird testing compat issue between FilePathsBase.jl and JLSO.jl. Honestly, might just drop the dependency since I don't think anyone outside of Invenia used that package anyway.

Longer term, I'm fine with moving the FilePathsBase.jl code over to FilePaths.jl. Also, might fit nicely with switching substring storage and bumping to 1.0 for easier compat handling.

@rofinn
Copy link
Owner

rofinn commented Mar 7, 2025

@frankier I've opened, merged and tagged a requires -> extensions PR on FilePaths.jl for now. Assuming the tag goes through, will that be enough to unblock you for now? I'll leave addressing the larger ecosystem / dependency issues to another night.

JuliaRegistries/General#126486

@frankier
Copy link
Author

frankier commented Mar 7, 2025

I think this should do the trick. We have to check if Makie loads faster but it should do. Thanks so much!

@frankier frankier closed this Mar 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants