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

[WIP] singular: 4.1.1p2 -> 4.1.1p3 #45439

Closed
wants to merge 1 commit into from

Conversation

timokau
Copy link
Member

@timokau timokau commented Aug 21, 2018

Motivation for this change

Adds proper tests to avoid something like #39370 in the future. Upstream is unclear about weather or not they are appropriate for packaging though. Since its a bit of a pain to contact upstream and they've been unresponsive for a while, I thought we may as well just try.

According to upstream, there may be difficulties with the tests running on 32 bit. Currently we don't compile on 32 bit anyways (reported that upstream too). As soon as we do, we can sort that out or disable some (or all if need be) tests.

Also removes the "enableFactory" option because singular actually enables factory by default and explicitly disabling it breaks the build. So the option was never really available.

@7c6f434c

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.

Adds proper tests. Also removes the "enableFactory" option because
singular actually enables factory by default and explicitly disabling it
breaks the build. So the option was never really available.
@7c6f434c
Copy link
Member

I dunno, maybe now that you have commit access you could just drop me from maintainer list of packages that I only touched because of Sage. Inside Nixpkgs Singular is only used by Sage, and in any topic related to Sage you know much more than me.

I wonder if there are non-Sage direct Nixpkgs users of Singular (for local development).

@GrahamcOfBorg
Copy link

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: singular

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.


@GrahamcOfBorg
Copy link

Failure on aarch64-linux (full log)

Attempted: singular

Partial log (click to expand)

mktexfmt: mktexfmt is using the following fmtutil.cnf file for writing changes:
mktexfmt:   /homeless-shelter/.texlive2017/texmf-config/web2c/fmtutil.cnf
/nix/store/90b59sc9hdlsflynd0pp7lw8i0hm08sb-texlive-combined-small-2017/bin/mktexfmt: mkdir(/homeless-shelter/) failed, goodbye: Permission denied
I can't find the format file `pdflatex.fmt'!
make[1]: *** [Makefile:525: cddlibman.pdf] Error 1
make[1]: Leaving directory '/build/source/doc'
make: *** [Makefile:445: all-recursive] Error 1
builder for '/nix/store/scczh7y1sc691i1zsfijwwa3fpa35yns-cddlib-0.94j.drv' failed with exit code 2
cannot build derivation '/nix/store/j4ikgjxq3j01g5jnj77hkiyhpf5bwyxp-singular-4.1.1p3.drv': 1 dependencies couldn't be built
error: build of '/nix/store/j4ikgjxq3j01g5jnj77hkiyhpf5bwyxp-singular-4.1.1p3.drv' failed

@GrahamcOfBorg
Copy link

Failure on x86_64-linux (full log)

Attempted: singular

Partial log (click to expand)

mktexfmt: mktexfmt is using the following fmtutil.cnf file for writing changes:
mktexfmt:   /homeless-shelter/.texlive2017/texmf-config/web2c/fmtutil.cnf
/nix/store/1phb2n8pfxf4adl21f178x5anr0j6gp4-texlive-combined-small-2017/bin/mktexfmt: mkdir(/homeless-shelter/) failed, goodbye: Permission denied
I can't find the format file `pdflatex.fmt'!
make[1]: *** [Makefile:525: cddlibman.pdf] Error 1
make[1]: Leaving directory '/build/source/doc'
make: *** [Makefile:445: all-recursive] Error 1
builder for '/nix/store/h8dlk92xlfviqnn1csh59dma1fvcjxcc-cddlib-0.94j.drv' failed with exit code 2
cannot build derivation '/nix/store/syzc1lxj7n0k913qw8ikp49wbifamzx8-singular-4.1.1p3.drv': 1 dependencies couldn't be built
error: build of '/nix/store/syzc1lxj7n0k913qw8ikp49wbifamzx8-singular-4.1.1p3.drv' failed

@timokau
Copy link
Member Author

timokau commented Aug 21, 2018

I don't know which packages you maintain for which reason, if you want that it is probably best you remove yourself. I think two maintainers are better than one, but I understand that your time is limited.

I think the ofborg failures are unrelated, I already encountered the cddlib failure somewhere else and am currently bisecting that.

@timokau
Copy link
Member Author

timokau commented Aug 21, 2018

(Bisecting needs llvm rebuilds though :/)

@7c6f434c
Copy link
Member

I said you could, not «you should», of course — I am OK with being listed as a maintainer and I will try to help as far as I can if someone has a problem with the package and chooses to ping me.

You can safely assume, though, that I trust your judgement more than my own for the Sage dependencies.

I am not sure what the mention is meant to mean in this case — maybe because I am used to (and I believe it is a reasonable approach to development given the scale) to direct pushes of changes to various packages, including non-maintainer updates to packages with a maintainer who just had not got around to bumping or fixing them.

Re: bisecting: I think we have a recent staging merge to make the bisection even less convenient…

@timokau
Copy link
Member Author

timokau commented Aug 21, 2018

Great :)

I just pinged you in case you because you are a maintainer. In case you care and/or have an opinion about the change. It is fine if you don't.

I generally prefer to go through PRs. In this case especially because I need ofBorg to see if the tests fail on other platforms. So I'll keep this open until the cddlib issue is resolved.

I already did a lot of git bisect skip hoping that I can get around the huge rebuilds but it looks like I'll just have to wait for them.

@7c6f434c
Copy link
Member

7c6f434c commented Aug 21, 2018 via email

@timokau
Copy link
Member Author

timokau commented Aug 22, 2018

It actually is due to #40826 (after rebuilding llvm 3 or 4 times -- I didn't know changes so close to the root of the dependency tree are that frequent).

@timokau
Copy link
Member Author

timokau commented Aug 24, 2018

@GrahamcOfBorg build singular

@GrahamcOfBorg
Copy link

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: singular

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.


@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: singular

Partial log (click to expand)

--- nchilb_test4            0.07
--- nchilb_test5            0.07
--- combinat                0.03
--- methods                 0.02
--- sets                    0.04
--- ssi_Z                   0.03
ok_s Summary: Checks:142 Failed:0 Time:179.12
Summary: Checks:594 Failed:0 Time:214.37
Exit status 0
/nix/store/xlp2m4h0i5jckka3226knndq16492rv5-singular-4.1.1p3

@GrahamcOfBorg
Copy link

Timed out, unknown build status on aarch64-linux (full log)

Attempted: singular

Partial log (click to expand)

--- bug_tr822               1.26
--- bug_tr834               1.17
--- bug_genus_etc           5.72
--- conv_bi                 6.18
--- facstd                  34.62
--- fermat_gcd_1var         4.34
--- fermat_gcd_2var         9.50
--- fermat_gcd_3var         50.18
building of '/nix/store/lhf6g8w5j3gn1x1h2fk8zrxyhfghsb52-singular-4.1.1p3.drv' timed out after 3600 seconds
error: build of '/nix/store/lhf6g8w5j3gn1x1h2fk8zrxyhfghsb52-singular-4.1.1p3.drv' failed

@timokau timokau changed the title singular: 4.1.1p2 -> 4.1.1p3 [WIP] singular: 4.1.1p2 -> 4.1.1p3 Aug 24, 2018
@timokau
Copy link
Member Author

timokau commented Aug 24, 2018

Waiting for sage support: https://trac.sagemath.org/ticket/25993#comment:21

@timokau timokau added the 2.status: wait-for-upstream Waiting for upstream fix (or their other action). label Aug 24, 2018
@7c6f434c
Copy link
Member

Does https://trac.sagemath.org/ticket/25993#comment:30 give us hope for the next Sage release?

@timokau
Copy link
Member Author

timokau commented Jan 21, 2019

Yes looks like the situation is better, but someone still has to work on it. I wish upstream wouldn't break on patch releases.

Anyway since this is taking longer than expected, we could update and then just pin the sage dependency if you have any usecase for the latest singular?

@7c6f434c
Copy link
Member

No hurry, I am just checking if any things GitHub shows me as pending (via its multiple inconsistent ways) are pending for transient reasons (some indeed were, but here we'll have to see what Sage upstream does)

@timokau
Copy link
Member Author

timokau commented Jan 21, 2019

Ah, alright. In this case I'm CC'd on the upstream ticket, so as soon as that is ready I will know.

@prusnak
Copy link
Member

prusnak commented Apr 29, 2020

Seems the latest is 4.1.2p1. Will you update this PR?

@timokau
Copy link
Member Author

timokau commented Apr 29, 2020

I'm still waiting for https://trac.sagemath.org/ticket/25993 to be resolved first. There has been recent progress, but I don't have the time to work on it directly right now. I'll just wait until the trac ticket is closed, then pull whatever patch they come up with into nixpkgs.

@ryantm ryantm marked this pull request as draft October 23, 2020 03:12
@collares collares mentioned this pull request Apr 20, 2021
14 tasks
@collares
Copy link
Member

An updated version of this patch (including the enableFactory and testrun changes) is in #116365, and should land together with Sage 9.3.

@timokau
Copy link
Member Author

timokau commented Apr 29, 2021

This is obsolete now that #116365 is merged. Thanks for the heads up @collares.

@timokau timokau closed this Apr 29, 2021
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

7 participants