Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mention Pkg does not support relative paths of packages in registries #43

Closed
wants to merge 2 commits into from

Conversation

jakobnissen
Copy link
Contributor

No description provided.

@jakobnissen
Copy link
Contributor Author

I also added a check to check whether the package repo is a relative path. This is tricky to check for, because frustratingly, there is no way to check if a string in Julia is a relative path. This is ultimately due to the weak typing Julia uses for paths :( For example, [email protected]/GunnarFarneback/LocalRegistry.jl/pull/43 is a valid local path on Unix.

So what the code does is to error if and only if the package path is an existing path, and it's not an absolute path. This will not catch the case where the package path is a relative path relative to the registry itself.

I'm not too happy with the solution, but it's hard to check for correctness when the repo can be both paths and URLs, and checking which is hard.

@@ -74,6 +74,7 @@ Notes:
* You need to have a clean working copy of your registry.
* The package must be stored as a git working copy, e.g. having been
cloned with `Pkg.develop`.
* Currently, relative paths of packages [are not supported](https:/JuliaLang/Pkg.jl/issues/677) by Pkg. Make sure to have `dev`'d an absolute path.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does it make a difference whether you dev an absolute path?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, I must have brainfarted.

# Check that `package_repo` is not a relative path. Relative paths do not work
# currently with Pkg (Pkg issue #677).
# This approach here does not always work, because the path may be relative to another
# location. Further, nearly all strings are valid paths in Unix, including URLs.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's rather dubious to check whether a repo path is relative to wherever you happen to have your current directory. How could Pkg possibly be expected to know where that is in order to make sense of the relative path?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah... maybe I should remove this check altogether. I wish it was possible to simply check if a string was a local path, but that is not possible because Julia does not provide a path type to disambiguate this.
The idea is that documentation is good, because it enables people to not screw up but throwing errors is better, because it forces them not to.

# This approach here does not always work, because the path may be relative to another
# location. Further, nearly all strings are valid paths in Unix, including URLs.
if ispath(package_repo) && !isabspath(package_repo)
error("Relative package paths are not supported")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not about "package path" but "package repo path".

@GunnarFarneback
Copy link
Owner

I have seen that you ran into troubles with relative local paths but generally speaking I don't think using local paths at all is a very common use case, although occasionally a useful option. It could be interesting to add a page under "Advanced Topics" which explains what you can and cannot do with registries and packages at local paths, and which could mention file:// URLs, which are maybe not commonly known.

@codecov-commenter
Copy link

codecov-commenter commented Aug 18, 2021

Codecov Report

Merging #43 (655315a) into master (50b6733) will decrease coverage by 0.53%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #43      +/-   ##
==========================================
- Coverage   98.31%   97.77%   -0.54%     
==========================================
  Files           1        1              
  Lines         178      180       +2     
==========================================
+ Hits          175      176       +1     
- Misses          3        4       +1     
Impacted Files Coverage Δ
src/LocalRegistry.jl 97.77% <50.00%> (-0.54%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50b6733...655315a. Read the comment docs.

@jakobnissen
Copy link
Contributor Author

Maybe it's uncommon to want to use relative paths, but I wouldn't think so. I would expect most users who wants a local registry stores it in a file system somewhere as opposed to host it on the Internet somewhere - after all, that's what makes it local, right? And in that case, it's more robust and easier to specify relative paths, see e.g. also the mentioned use case in JuliaLang/Pkg.jl#677.

But whether or not it's common, it's certainly a predictable use case. That is, with near 100% certainty, more people are going to want to do this. I'm sure they will eventually find out the gotcha's, like that URLs can also be a path to a git repo, and it fails if you specify relative package paths, but it would just be much easier to somehow prevent them from getting those problems. Either by docs, or preferably by actually making it error.

@GunnarFarneback
Copy link
Owner

I would expect most users who wants a local registry stores it in a file system somewhere as opposed to host it on the Internet somewhere - after all, that's what makes it local, right?

For what it's worth, no it doesn't. I'm rather thinking of local organizations which might make their registry public on the internet or private on an internal VPN but in either case primarily targeted for their internal use. Maybe it's still more common than I believe to share resources on a filesystem but the trend I've seen is strongly toward web services, which in particular makes a lot of sense for git repositories.

@GunnarFarneback
Copy link
Owner

Same annoying CI failure as in #41.

@GunnarFarneback
Copy link
Owner

Closed in favor of #66.

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