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

fplll: 5.2.1 -> 5.3.0, python.pkgs.fpylll: 0.4.1dev -> 0.5.0dev #74372

Merged
merged 6 commits into from Dec 12, 2019

Conversation

r-ryantm
Copy link
Contributor

Semi-automatic update generated by https://github.com/ryantm/nixpkgs-update tools. This update was made based on information from https://repology.org/metapackage/fplll/versions.

meta.description for fplll is: '"Lattice algorithms using floating-point arithmetic"'.

meta.homepage for fplll is: '""

Compare changes on GitHub

Checks done (click to expand)
Rebuild report (if merged into master) (click to expand)

18 total rebuild path(s)

6 package rebuild(s)

6 x86_64-linux rebuild(s)
4 i686-linux rebuild(s)
4 x86_64-darwin rebuild(s)
4 aarch64-linux rebuild(s)

First fifty rebuilds by attrpath
fplll
python27Packages.fpylll
python37Packages.fpylll
python38Packages.fpylll
sage
sageWithDoc

Instructions to test this update (click to expand)

Either download from Cachix:

nix-store -r /nix/store/00q56pdlsnpyj8mwjwf0xd2qzwl5ixwr-fplll-5.3.0 \
  --option binary-caches 'https://cache.nixos.org/ https://r-ryantm.cachix.org/' \
  --option trusted-public-keys '
  r-ryantm.cachix.org-1:gkUbLkouDAyvBdpBX0JOdIiD2/DP1ldF3Z3Y6Gqcc4c=
  cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=
  '

(r-ryantm's Cachix cache is only trusted for this store-path realization.)

Or, build yourself:

nix-build -A fplll https://github.com/r-ryantm/nixpkgs/archive/72172b0f9d1e3f464757a2e1b6e02b17c4f91cff.tar.gz

After you've downloaded or built it, look at the files and if there are any, run the binaries:

ls -la /nix/store/00q56pdlsnpyj8mwjwf0xd2qzwl5ixwr-fplll-5.3.0
ls -la /nix/store/00q56pdlsnpyj8mwjwf0xd2qzwl5ixwr-fplll-5.3.0/bin

cc @7c6f434c for testing.

@7c6f434c
Copy link
Member

7c6f434c commented Dec 1, 2019

@timokau probably has a qualified opinion

@timokau
Copy link
Member

timokau commented Dec 1, 2019

I don't have any opinion other than nix-review should pass -- which it currently doesn't do since this breaks fpyll. Those two probably need to be updated together.

@7c6f434c
Copy link
Member

7c6f434c commented Dec 1, 2019

Ah right, I guess you only start forming opinion once Sage is the only thing that fails.

@timokau
Copy link
Member

timokau commented Dec 1, 2019

Well that's included in nix-review passing ;)

What I meant is that I don't personally use this package, so I don't really have anything to add in addition to "it shouldn't obviously break anything".

@7c6f434c
Copy link
Member

7c6f434c commented Dec 1, 2019

Well, you might have already heard of some complaints about the new version breaking Sage. (And no other use of this package seems to be known)

@timokau
Copy link
Member

timokau commented Dec 1, 2019

Good point :) There don't seem to be any known issues. Other than that, I usually trust the testsuite. It tends to have lots of false-positives, but rarely false-negatives.

@timokau
Copy link
Member

timokau commented Dec 2, 2019

Fpyll update is not trivial, something has changed with the test setup. I've bisected it to either 327c4c1a3f86c0fc877cfe3e177a3ab3664ece5b or 9732fdb40cf1bd43ad1f60762ec0a8401743fc79.

No time to continue right now. I'll try to get back to it when I have time, but if someone else wants to finish I certainly don't mind ;)

@timokau
Copy link
Member

timokau commented Dec 2, 2019

Okay the fpylll build succeeds now. There is one transient test issue (fplll/fpylll#161) that I've skipped for now. We should give upstream a couple of days to react to that before merging.

I haven't run nix-review yet.

@ofborg ofborg bot requested a review from timokau December 2, 2019 12:12
@timokau timokau changed the title fplll: 5.2.1 -> 5.3.0 fplll: 5.2.1 -> 5.3.0, python.pkgs.fpylll: 0.4.1dev -> 0.5.0dev Dec 2, 2019
@timokau
Copy link
Member

timokau commented Dec 2, 2019

nix-review passes.

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise LGTM

pkgs/development/libraries/fplll/default.nix Outdated Show resolved Hide resolved
@timokau
Copy link
Member

timokau commented Dec 9, 2019

Should be good to go now if @7c6f434c approves of the cleanup commits. If you still prefer the more concise style you used when you created the package, I can revert that change. I just thought that commits like the "clean up dependencies" one have a nicer diff with the new formatting.

The changelog is not ideal, since it links to a search. The project also has a proper changelog, but that has been out of date for a couple of years.

According to my perception of the current community standards.
autoreconfHook implies autotools.
@timokau
Copy link
Member

timokau commented Dec 11, 2019

The failing test is now resolved as well.

@7c6f434c
Copy link
Member

As I consider you more competent than myself here (because of your great Sage work), I don't have any opinions re: style.

Re: changelog — the pain, it's not just search, it is JavaScript-only search… I propose https://github.com/fplll/fplll/releases as the first entry in the changelog list.

(I have read the entire diff and briefly checked the basic reasonability of the added patch)

The two packages need to be updated together, since fpylll 0.4.1
requires the old fplll version and fpylll 0.5.0 requires the new one.
@timokau
Copy link
Member

timokau commented Dec 12, 2019

As I consider you more competent than myself here (because of your great Sage work), I don't have any opinions re: style.

Appreciate the compliment :)

Re: changelog — the pain, it's not just search, it is JavaScript-only search… I propose https://github.com/fplll/fplll/releases as the first entry in the changelog list.

Oh neat, I didn't realise the git tags had release notes as well. I've added that to the list (although they don't seem complete -- for example the latest version doesn't have any release notes there).

(I have read the entire diff and briefly checked the basic reasonability of the added patch)

Please have another look (just changed the changelogs) and merge if it looks good to you.

@7c6f434c
Copy link
Member

I wonder at what point we will just subscribe a bot to various mailing list with release announcements because it is more reliable…

@7c6f434c 7c6f434c merged commit c2ae05d into NixOS:master Dec 12, 2019
@timokau
Copy link
Member

timokau commented Dec 16, 2019

Unfortunately this PR broke the aarch64 build of fpylll: fplll/fpylll#162

Though the test failures look pretty harmless, so we could probably just ignore them.

@r-ryantm r-ryantm deleted the auto-update/fplll branch December 22, 2019 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants