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.

Sign up for GitHub

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

Jump to bottom

isl: add 0.20 #50642

Merged
merged 1 commit into from Nov 19, 2018
Merged

isl: add 0.20 #50642

merged 1 commit into from Nov 19, 2018

Conversation

bts
Copy link
Contributor

@bts bts commented Nov 18, 2018

Motivation for this change

I need version 0.20 for a project, and I noticed that only 0.17.1 is in nixpkgs.

There are now many versions of this library in nixpkgs:

  • isl_0_11 - gcc49 and cloog_0_18_0 depend on this
  • isl_0_12 - no dependents in nixpkgs
  • isl_0_14 - cloog, gcc48, gcc5, gcc6, and llvmPackages_35 depend on this
  • isl_0_15 - no dependents in nixpkgs
  • isl_0_17 - gccCrossStageStatic, gcc7, gcc8, and gcc-snapshot depend on this. additionally the canonical isl currently points to this
  • isl_0_20 - from this PR. corresponds to the latest release of the library.

Questions of which I am unsure:

  • Should the versions of the library with no dependents (i.e. isl_0_12 and isl_0_15) be removed? or should we keep them around, in case packages that currently depend on earlier versions (e.g. gcc49, which depends on isl_0_11) can and should be upgraded to a slightly newer version of the library (e.g. isl_0_12) in the future?
  • With packages as important as gcc depending on isl, is it appropriate to change isl to point to this latest version, isl_0_20?

See previous/related discussion here: #21066. As for testing this, I have been using this derivation in a project that successfully links against the library.

Please let me know how I can help from here.

Thanks!
Brian

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@bts bts mentioned this pull request Nov 18, 2018
Merged
@Mic92
Copy link
Member

Mic92 commented Nov 18, 2018

Versions with no dependent packages of isl can be removed. If upgrading isl would work for gcc, we should do this.

@bts
Copy link
Contributor Author

bts commented Nov 19, 2018

I removed isl_0_12 and isl_0_15 due to the lack of dependent packages and upgraded isl to use isl_0_20 by default. I tried bumping the isl dependency from isl_0_17 to isl_0_20 in gcc8 but this failed to build, so I left all of the gccs alone.

@bts
Copy link
Contributor Author

bts commented Nov 19, 2018

I ran nix-shell -p nox --run "nox-review wip" but this didn't appear to rebuild all packages that use isl?

==> We're in a git repo, trying to fetch it                                                                                                                                                                                                                                                                          
Building in /run/user/1001/nox-review-8zisflqa: isl_0_17 isl                                                                                                                                                                                                                                                         
Invoking nix-build -A isl_0_17 -A isl /home/bts/code/nixpkgs                                                                                                                                                                                                                                                         
these paths will be fetched (0.64 MiB download, 2.37 MiB unpacked):                                                                                                                                                                                                                                                  
  /nix/store/wkn7b37wwr1ggjmaq0f8jng69vy9j812-isl-0.17.1                                                                                                                                                                                                                                                             
copying path '/nix/store/wkn7b37wwr1ggjmaq0f8jng69vy9j812-isl-0.17.1' from 'https://cache.nixos.org'...                                                                                                                                                                                                              
/nix/store/wkn7b37wwr1ggjmaq0f8jng69vy9j812-isl-0.17.1                                                                                                                                                                                                                                                               
/nix/store/8pr2m3kvzh5a6zh4nd12ydnlyw6bkrhv-isl-0.20                                                                                                                                                                                                                                                                 
Result in /run/user/1001/nox-review-8zisflqa                                                                                                                                                                                                                                                                         
total 0                                                                                                                                                                                                                                                                                                              
lrwxrwxrwx 1 bts users 54 Nov 19 09:51 result -> /nix/store/wkn7b37wwr1ggjmaq0f8jng69vy9j812-isl-0.17.1                                                                                                                                                                                                              
lrwxrwxrwx 1 bts users 52 Nov 19 09:51 result-2 -> /nix/store/8pr2m3kvzh5a6zh4nd12ydnlyw6bkrhv-isl-0.20

@Mic92
Copy link
Member

Mic92 commented Nov 19, 2018

Apparently there is no package that uses the default isl package.

@bts
isl: add 0.20
0e8c811 
Update `isl` to use `isl_0_20`.
@Mic92 Mic92 merged commit 20b2253 into NixOS:master Nov 19, 2018
@bts bts deleted the isl_0_20 branch November 19, 2018 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mic92 Mic92 Mic92 left review comments

Assignees
No one assigned
Labels
8.has: clean-up 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@bts @Mic92 @GrahamcOfBorg