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

Pkg.add operates in-place on the arguments without notice #3112

Open
giordano opened this issue Jun 16, 2022 · 4 comments
Open

Pkg.add operates in-place on the arguments without notice #3112

giordano opened this issue Jun 16, 2022 · 4 comments
Labels
binarybuilder Related to the use of Pkg in BinaryBuilder

Comments

@giordano
Copy link
Contributor

giordano commented Jun 16, 2022

julia> using Pkg

julia> Pkg.activate(; temp=true, io=devnull)

julia> spec = Pkg.PackageSpec(; name = "Example", version = Pkg.Types.VersionSpec("0.5.3"));

julia> Pkg.add(Pkg.Types.Context(), [spec]; io=devnull)

julia> spec
PackageSpec(
  name = Example
  uuid = 7876af07-990d-54b4-ab0e-23690620f79a
  tree_hash = 46e44e869b4d90b96bd8ed1fdcf32244fddfb6cc
  version = v"0.5.3"
)

spec changed after the call to Pkg.add, it acquired the tree hash and the UUID. One could claim this method is internal, but this still violates the convention of appending ! to a function name when arguments are modified in-place.

@KristofferC
Copy link
Member

pkgs = deepcopy(pkgs) # don't mutate input

@giordano
Copy link
Contributor Author

That's not the same method I used above.

@IanButterworth
Copy link
Member

The rub again is that the first arg Context() methods aren't part of the public API. And adding them was rejected #2952

So seems like you need to deepcopy before passing the arg in to use this internal method

@giordano
Copy link
Contributor Author

The rub again is that the first arg Context() methods aren't part of the public API.

In fact I said

One could claim this method is internal, but this still violates the convention of appending ! to a function name when arguments are modified in-place.

So seems like you need to deepcopy before passing the arg in to use this internal method

Which is what I had done before opening the issue: JuliaPackaging/BinaryBuilderBase.jl#250. But discovering that a method not following the naming convention was modifying my specs, just to make me run into another Pkg bug #3113, wasn't exactly fun

@giordano giordano added the binarybuilder Related to the use of Pkg in BinaryBuilder label Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
binarybuilder Related to the use of Pkg in BinaryBuilder
Projects
None yet
Development

No branches or pull requests

3 participants