-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Remove package extensions in Pkg functions #20681
Conversation
Thanks for working on this! As a test, you could add/remove the Example package both with and without the .jl suffix. |
test/pkg.jl
Outdated
Pkg.status("Example.jl", iob) | ||
str = chomp(String(take!(iob))) | ||
@test endswith(str, string(Pkg.installed("Example.jl"))) | ||
@test isempty(Pkg.dependents("Example.jl")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems a little fragile, anyone could break this by adding a package that depends on Example in the future (which is kinda silly, but allowed)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just copied from the usual test suite:
Line 127 in 1297ede
@test isempty(Pkg.dependents("Example")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But yeah, both tests seem fragile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems pretty hypothetical. If anyone does that, we can fix the tests then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would immediately break the tests of all past versions of julia though, which isn't great. Would be better to move this to a section of the tests that's running against a known past commit of metadata, rather than the latest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to push a commit to this branch fixing this and the test linked in my comment above or handle it in a separate PR. I am not sure how I should go about it.
base/pkg/pkg.jl
Outdated
@@ -37,6 +37,10 @@ const cd = Dir.cd | |||
|
|||
dir(path...) = Dir.path(path...) | |||
|
|||
# remove extension .jl | |||
const PKGEXT = ".jl" | |||
splitjl(pkg::AbstractString) = endswith(pkg, PKGEXT) ? splitext(pkg)[1] : pkg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that it really matters but pkg[1:(end - 3)]
would be much faster...
doc/src/manual/packages.md
Outdated
@@ -171,6 +171,14 @@ git config --global url."https://".insteadOf git:https:// | |||
|
|||
However, this change will be system-wide and thus the use of [`Pkg.setprotocol!()`](@ref) is preferable. | |||
|
|||
!!! note | |||
The package manager functions accept package names with the `.jl` extension: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd probably word this to say that it's possible but is stripped out internally so as not to encourage this style of use. Maybe "The package manager functions also accept the .jl
suffix on package names, though the suffix is stripped out internally."
a469e2d
to
c3052a1
Compare
Failures seems unrelated? Feel free to restart. |
Restarded travis, log at: https://gist.github.com/KristofferC/162cbe28353847ad4197e5e9c2337454 |
to avoid fragility in the case that any future package happens to depend on Example.jl
Bump. (Too late for this to be merged?) |
This implements #20630 and allows for eg.
by stripping of the terminating
.jl
extension.TODO:
fix #20630