Skip to content

Conversation

@IanButterworth
Copy link
Member

@IanButterworth IanButterworth commented Nov 7, 2021

The compat field of a Project appears to breaking equality even when the projects are identical.

julia> Pkg.Types.Compat(Pkg.Types.VersionSpec("0.1"), "0.1") == Pkg.Types.Compat(Pkg.Types.VersionSpec("0.1"), "0.1")
false

This was observed in JuliaRegistries/General#47659 (comment) where an unchanged Project was being re-written to disk, without any actual changes to the file even though this check should prevent that

I'm adding the tests first to prove the problem exists first.

Also, @staticfloat the pkgserver tests are failing with

┌ Warning: could not download https://pkg.julialang.org/registries
│   exception = HTTP/1.1 502 Bad Gateway while requesting https://pkg.julialang.org/registries
└ @ Pkg.Registry ~/Documents/GitHub/Pkg.jl/src/Registry/Registry.jl:66

@staticfloat
Copy link
Member

PkgServer breakage has been fixed.

@IanButterworth IanButterworth merged commit 05d828d into JuliaLang:master Nov 7, 2021
@IanButterworth IanButterworth deleted the ib/project_equality branch November 7, 2021 19:34
@fredrikekre
Copy link
Member

Is this equality check used for something other than #2202 ? That PR didn't implement it so I guess so? Possibly another internal function should be used instead of overloading == and hash then.

@IanButterworth
Copy link
Member Author

Sorry, I don't follow. I may be being slow though..

This PR specifically fixes a bug in the behavior introduced in #2202 where it will alway re-write a Project when compat fields are present. And because the equality check #2202 uses is just ==(::Project, ::Project) I don't see how fixing this could be anything other than defining == and hash for Compat?

@fredrikekre
Copy link
Member

I just meant that probably == as used in #2202 could be switched to use something like have_same_toml_representation(p1, p2)::Bool function instead. But since #2202 didn't add the equality check maybe it is used for something else?

KristofferC pushed a commit that referenced this pull request Nov 9, 2021
@KristofferC
Copy link
Member

I don't think this applies to 1.6.

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.

4 participants