-
Notifications
You must be signed in to change notification settings - Fork 59
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 Pkg from dependencies #1896
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1896 +/- ##
==========================================
- Coverage 87.25% 87.24% -0.01%
==========================================
Files 97 97
Lines 35655 35653 -2
==========================================
- Hits 31111 31106 -5
- Misses 4544 4547 +3 ☔ View full report in Codecov by Sentry. |
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 does not work on julia 1.6 (the compat bound of Nemo)
Would it be reasonable to do EDIT: ah, no, the old version loads Pkg. |
|
version() = VersionNumber("$(ver.version)") | ||
end | ||
end | ||
version() = Base.get_pkgversion_from_path(pkgdir) |
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.
If this was just about the Package version, we could use the Compat
package and then Compat.pkgversion(Nemo)
.
However this code also tries to determine if one is using a dev
version, i.e., whether it is being run from a git
clone. Alas the code for doing that is buggy already (it checks whether /dev/
is in the path, which is OK if someone used Pkg.develop
but not for people like me who manually git clone
repositories).
Interestingly Nemo.versioninfo
tries to get a more thorough git
status, but it is also broken (at least with Julia 1.10 and 1.11)
julia> Nemo.versioninfo()
Nemo version 0.47.1
Nemo: ERROR: UndefVarError: `LibGit2` not defined in `Base`
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.
In Oscar.jl we instead use this code to determine if it is a dev version (but this is also far from perfect -- and it uses Pkg
):
const is_dev = (function()
deps = Pkg.dependencies()
if Base.haskey(deps, PROJECT_UUID)
if deps[PROJECT_UUID].is_tracking_path
return true
end
end
return occursin("-dev", lowercase(string(VERSION_NUMBER)))
end)()
I see, thank you for explanation. In case Otherwise, when |
@sumiya11 I have no idea and in any case, none of this is documented AFAIK. I am not concerned with imitating current behavior 1-1. Rather, the general idea, which is to be able to detect reliably which version a user reporting an issue really is using: i.e. either a specific stable release, and which; or some git version (ideally identified by commit). Just like the Julia banner does (perhaps one could look at its current code for inspiration). |
To be clear: In general I would be very happy if we could drop the dependency on |
Thank you for the clarification. If so, why not ask the user for the output of |
It is mainly for the banner which includes the version number. Maybe they fix the Pkg loading regression in 1.11.1. It is not the only thing that got slower. |
A PR for #1895.
I don't know if
Base.get_pkgversion_from_path
covers all the cases that were previously handled.