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

buildGoPackage: use $out instead of $bin #85535

Closed
wants to merge 3 commits into from
Closed

buildGoPackage: use $out instead of $bin #85535

wants to merge 3 commits into from

Conversation

zowoq
Copy link
Contributor

@zowoq zowoq commented Apr 19, 2020

cc @kalbasit @Mic92

This changes buildGoPackage to use to $out instead of $bin which matches the behaviour of buildGoModule (and buildRustPackage).

# Unfortunately we have to keep an empty reference to $out, because it seems
# buildGoPackages only nukes references to the go compiler under $bin, effectively
# making all binary output under $bin mandatory. Ideally, we would just use
# $out and $man and remove $bin since there's no point in an empty path. :(
outputs = [ "bin" "man" "out" ];

@adisbladis
Copy link
Member

In #43328 I opted not to do this as it would introduce an incompatibility for questionable gain.

@kalbasit
Copy link
Member

Can we alias out to bin for backward compatibility?

@Mic92
Copy link
Member

Mic92 commented Apr 19, 2020

Can we alias out to bin for backward compatibility?

It could be a symlink I think. However I suspect most of the breakages occur in the build expressions itself because $bin needs to be changed to $out.

@kalbasit
Copy link
Member

How about deprecate the output (not sure that's possible, at least the trace part), spread the word to give people a couple of months before we do this? In the meantime we could gate the output change with an attr and set it from within nixpkgs.

@zowoq
Copy link
Contributor Author

zowoq commented Apr 28, 2020

I might do another round of $bin cleanup but starting to get to the point of diminishing returns.

I'm not sure how to move forward with this?

@Mic92
Copy link
Member

Mic92 commented Apr 28, 2020

The mkdir in nixpkgs/pkgs/servers/monitoring/uchiwa/default.nix needs a -p.

@zowoq
Copy link
Contributor Author

zowoq commented Apr 28, 2020

It already has -p?

@zowoq
Copy link
Contributor Author

zowoq commented Apr 28, 2020

The other mkdir. Fixed.

@Mic92
Copy link
Member

Mic92 commented Apr 28, 2020

Could you also write a short entry in doc/manual/release-notes/rl-2009.xml. This is a breaking change for all out-of-tree buildGoPackages.

@zowoq
Copy link
Contributor Author

zowoq commented Apr 28, 2020

Added a note and dropped a paragraph from the go docs that referenced defaulting to $bin.

Mic92 added a commit that referenced this pull request Apr 28, 2020
buildGoPackage: use $out instead of $bin
@Mic92
Copy link
Member

Mic92 commented Apr 28, 2020

Merged into staging in 20e67f1

@Mic92 Mic92 closed this Apr 28, 2020
Staging automation moved this from Needs review to Done Apr 28, 2020
@zowoq zowoq deleted the gopackage branch April 28, 2020 11:59
@Mic92 Mic92 mentioned this pull request May 11, 2020
10 tasks
Mic92 pushed a commit that referenced this pull request May 11, 2020
orivej added a commit to orivej/fptrace that referenced this pull request May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Staging
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants