(compilers): add version and pname #67284

gloaming · 2019-08-22T18:53:10Z

Motivation for this change

It's annoying to have to use getVersion with clang and friends. Might as well add the pname while we're at it.

Things done

I ran the rebuild for a little bit (on NixOS) to check it wasn't totally broken.

Notify maintainers

cc @

pkgs/development/compilers/gcc/4.8/default.nix

oxij · 2019-08-23T10:03:15Z

@oxij I see you moved the `crossNameAddon` to the start for the earlier versions. Are you aware of any reason I shouldn't change it for the later ones also?

No, I just didn't touch the expressions for the later `gcc`s then. So, if I were you, I would change them now.

gloaming · 2019-08-25T21:04:23Z

Thanks. pname now added to gcc.

Note that the cc-wrapper doesn't forward the pname; I'm not sure if it's worth the hassle.

teto · 2019-08-28T15:28:48Z

needs a rebase

veprbl · 2019-10-25T19:54:41Z

pkgs/build-support/cc-wrapper/default.nix

@@ -35,8 +35,8 @@ let
  targetPrefix = stdenv.lib.optionalString (targetPlatform != hostPlatform)
                                           (targetPlatform.config + "-");

-  ccVersion = (builtins.parseDrvName cc.name).version;
-  ccName = (builtins.parseDrvName cc.name).name;
+  ccVersion = cc.version or (builtins.parseDrvName cc.name).version;


How about

Suggested change

ccVersion = cc.version or (builtins.parseDrvName cc.name).version;

ccVersion = lib.getVersion cc;

Good point. I wonder if we should add a lib.getPName as well?

@gloaming I think that this goes out of scope of this PR.

veprbl · 2019-10-25T20:08:07Z

pkgs/development/compilers/gcc/8/default.nix


    bootstrap = targetPlatform == hostPlatform;

 in

 stdenv.mkDerivation ({
-  name = "${name}${if stripped then "" else "-debug"}-${version}" + crossNameAddon;
+  pname = crossNameAddon + "${name}${if stripped then "" else "-debug"}";


I see we were missing this as 2afd3c9 and 659363f must have crossed.

I messed this up recently too. Maybe we can finally fix it in this PR.

pkgs/development/compilers/gcc/8/default.nix

pkgs/development/compilers/gcc/9/default.nix

pkgs/development/compilers/gcc/snapshot/default.nix

gloaming · 2019-11-23T19:00:54Z

@veprbl Suggestions added.

Ericson2314

Would it be possible to base your PR on top of git merge-base origin/master origin/release-19.09? I would very much appreciate that.

I have been making a number of cleanups to both master and release-19.09, and will probably do some more, and it is much easier to "chain" them when I can just merge the same commit into both branches (or rather staging and staging -19.09).

I'm not sure this can all go to 19.09, but there is at least some parts which can, and I would add those parts at the very least, which does mean I'd need to make another commit, but the base branch per the above still helps

veprbl · 2019-11-23T19:20:13Z

pkgs/build-support/cc-wrapper/default.nix

@@ -96,6 +96,7 @@ stdenv.mkDerivation {
  name = targetPrefix
    + (if name != "" then name else stdenv.lib.removePrefix targetPrefix "${ccName}-wrapper")
    + (stdenv.lib.optionalString (cc != null && ccVersion != "") "-${ccVersion}");
+  version = ccVersion;


On the line above we conditionally append "-${ccVersion}" suffix and here you refer to ccVersion unconditionally. I wonder what is the use case for cc-wrapper with cc = null, depending on what it is we might not want to break it (ccVersion tries to evaluate cc.name).

cc == null is for this native stdenv stuff I think we should remove anyways.

OK, I've added a null check. We still have the minor issue that a null cc will result in a null version attribute rather than a missing one, which seems to be against convention. I don't know an elegant way to conditionally add an attribute to the derivation...

Great! Yeah this fine for now / master.

pkgs/development/compilers/gcc/8/default.nix

Ericson2314 · 2019-11-23T19:28:25Z

BTW, I like to do

vim -d pkgs/development/compilers/gcc/*/default.nix

to make sure all this copy-pasted code isn't drifting apart.

and fix the dashes

gloaming · 2019-11-23T22:01:16Z

Would it be possible to base your PR on top of git merge-base origin/master origin/release-19.09? I would very much appreciate that.

Remarkably, it rebased without any conflict :)

Ericson2314 · 2019-11-23T23:07:29Z

Oh! Last thing there is a gcc/snapshot/default.nix you should get too, even though it should also be deleted.

gloaming · 2019-11-24T00:01:02Z

Oh! Last thing there is a gcc/snapshot/default.nix you should get too, even though it should also be deleted.

Hmm, I made the same changes there as to the others - is there something I missed?

Ericson2314 · 2019-11-24T05:07:18Z

Oh, I'm so sorry @gloaming! I was reviewing earlier today from my phone and somehow missed that file. Thanks again for your work and patience.

veprbl · 2019-11-24T05:09:05Z

I'm very confused. Why is this going to release-19.09?

Ericson2314 · 2019-11-24T05:25:07Z

@veprbl Oh. shit! I meant for this to still go to staging, sorry if that was unclear.

Ericson2314 · 2019-11-24T05:26:46Z

I'll revert.

This PR was meant to go to staging, not 19.09. I was not clear. This reverts commit ba62f5e, reversing changes made to c02134f.

jonringer · 2019-11-24T05:32:54Z

for #74014 I was wondering why my previous nix-review only built ~100 packages, then ~900 a few minutes later... ty for reverting 😄

Ericson2314 · 2019-11-24T05:34:56Z

Hehe NP.

gloaming requested review from Ericson2314 and matthewbauer as code owners August 22, 2019 18:53

jonringer reviewed Aug 22, 2019

View reviewed changes

pkgs/development/compilers/gcc/4.8/default.nix Outdated Show resolved Hide resolved

ofborg bot added 10.rebuild-darwin-stdenv 10.rebuild-linux-stdenv labels Aug 22, 2019

ofborg bot requested review from peti, 7c6f434c, dtzWill, lovek323 and veprbl August 22, 2019 19:52

ofborg bot added 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ labels Aug 22, 2019

gloaming force-pushed the cc-versions branch 2 times, most recently from 731c848 to dc550f7 Compare August 29, 2019 15:31

veprbl reviewed Oct 25, 2019

View reviewed changes

veprbl added this to Needs review in Staging Oct 25, 2019

veprbl reviewed Oct 25, 2019

View reviewed changes

pkgs/development/compilers/gcc/8/default.nix Outdated Show resolved Hide resolved

veprbl reviewed Oct 25, 2019

View reviewed changes

pkgs/development/compilers/gcc/9/default.nix Outdated Show resolved Hide resolved

veprbl reviewed Oct 25, 2019

View reviewed changes

pkgs/development/compilers/gcc/snapshot/default.nix Outdated Show resolved Hide resolved

veprbl moved this from Needs review to WIP in Staging Oct 27, 2019

gloaming force-pushed the cc-versions branch from dc550f7 to 43afe78 Compare November 23, 2019 18:58

Ericson2314 requested changes Nov 23, 2019

View reviewed changes

veprbl reviewed Nov 23, 2019

View reviewed changes

Ericson2314 reviewed Nov 23, 2019

View reviewed changes

pkgs/development/compilers/gcc/8/default.nix Outdated Show resolved Hide resolved

gloaming force-pushed the cc-versions branch from 43afe78 to 607f947 Compare November 23, 2019 21:56

gloaming added 10 commits November 23, 2019 21:59

gcc: add version attribute

d7fd7ff

clang: add version attribute

c4a81ed

llvm: add version attribute

7808398

cc-wrapper: use cc pname/version if set

591034b

cc-wrapper: inherit compiler version

b3a1808

clang: use pname

b9f4896

clang-manpages: use pname

a6c8c28

llvm: use pname

9b3f3b8

llvm-manpages: use pname

2b616d2

gcc: use pname

e158e11

and fix the dashes

gloaming changed the base branch from staging to release-19.09 November 23, 2019 22:04

ofborg bot requested a review from veprbl November 23, 2019 22:05

gloaming force-pushed the cc-versions branch from 607f947 to e158e11 Compare November 23, 2019 22:06

Ericson2314 merged commit ba62f5e into NixOS:release-19.09 Nov 24, 2019

Staging automation moved this from WIP to Done Nov 24, 2019

Ericson2314 added a commit that referenced this pull request Nov 24, 2019

Revert "Merge pull request #67284 from gloaming/cc-versions"

d70000a

This PR was meant to go to staging, not 19.09. I was not clear. This reverts commit ba62f5e, reversing changes made to c02134f.

Ericson2314 mentioned this pull request Nov 24, 2019

(compilers): add version and pname #74017

Closed

10 tasks

Ericson2314 mentioned this pull request Nov 24, 2019

compilers, binutils: Add version and pname #74065

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(compilers): add version and pname #67284

(compilers): add version and pname #67284

gloaming commented Aug 22, 2019

oxij commented Aug 23, 2019 via email

gloaming commented Aug 25, 2019

teto commented Aug 28, 2019

veprbl Oct 25, 2019

gloaming Nov 23, 2019

veprbl Nov 23, 2019

veprbl Oct 25, 2019

Ericson2314 Nov 23, 2019

gloaming commented Nov 23, 2019

Ericson2314 left a comment •

edited

veprbl Nov 23, 2019

Ericson2314 Nov 23, 2019 •

edited

gloaming Nov 23, 2019

Ericson2314 Nov 23, 2019

Ericson2314 commented Nov 23, 2019

gloaming commented Nov 23, 2019

Ericson2314 commented Nov 23, 2019

gloaming commented Nov 24, 2019

Ericson2314 commented Nov 24, 2019 •

edited

veprbl commented Nov 24, 2019

Ericson2314 commented Nov 24, 2019

Ericson2314 commented Nov 24, 2019

jonringer commented Nov 24, 2019

Ericson2314 commented Nov 24, 2019

	ccVersion = cc.version or (builtins.parseDrvName cc.name).version;
	ccVersion = lib.getVersion cc;

(compilers): add version and pname #67284

(compilers): add version and pname #67284

Conversation

gloaming commented Aug 22, 2019

Motivation for this change

Things done

Notify maintainers

oxij commented Aug 23, 2019 via email

gloaming commented Aug 25, 2019

teto commented Aug 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gloaming commented Nov 23, 2019

Ericson2314 left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ericson2314 Nov 23, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ericson2314 commented Nov 23, 2019

gloaming commented Nov 23, 2019

Ericson2314 commented Nov 23, 2019

gloaming commented Nov 24, 2019

Ericson2314 commented Nov 24, 2019 • edited

veprbl commented Nov 24, 2019

Ericson2314 commented Nov 24, 2019

Ericson2314 commented Nov 24, 2019

jonringer commented Nov 24, 2019

Ericson2314 commented Nov 24, 2019

Ericson2314 left a comment •

edited

Ericson2314 Nov 23, 2019 •

edited

Ericson2314 commented Nov 24, 2019 •

edited