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

sage: 6.8 -> 8.0 #31298

Merged
merged 2 commits into from Nov 6, 2017
Merged

sage: 6.8 -> 8.0 #31298

merged 2 commits into from Nov 6, 2017

Conversation

timokau
Copy link
Member

@timokau timokau commented Nov 5, 2017

This "un-breaks" sage while also updating it to 8.0.

It compiles sage with its dependencies as one big pile, which is not
the best approach but definately better than nothing for now.

To be able to shrink the huge output pile a little, it also splits
docs from the rest of the output.

A fair warning for anybody who would like to test this: The final test build
took 2h 45min on my laptop, with the main package taking up 2.1G and the
docs 1.8GB.

As mentioned in the comments there are some things that could be solved
better, but as it already took more time than expected and I don't know
when or if I will find that time I thought I'd push something working now.

I actually started writing a script to convert the spkg declarations to
nix packages. Thanks to nix we could pin down all those dependencies to the
exact versions used upstream and those versions could co-exist with the newest
versions. It would allow gradual picking of dependencies and overwriting single
dependencies. I'm not sure if I will go through with this though.

Fixes #27335. Also we should close #15732.

Motivation for this change

Sage has been broken and outdated for a while.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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/)
  • Fits CONTRIBUTING.md.

I tested the execution of bin/sage, but not the other executables as most are irrelevant and/or I don't know what they do.

This "un-breaks" sage while also updating it to 8.0.

It compiles sage with its dependencies as one big pile, which is not
the best approach but definately better than nothing for now.

To be able to shrink the huge output pile a little, it also splits
docs from the rest of the output.
license = stdenv.lib.licenses.gpl2Plus;
broken = true;
platforms = stdenv.lib.platforms.all;
Copy link
Member

Choose a reason for hiding this comment

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

Do you actually expect this to build on Darwin? And on Cygwin? I would put linux for now…

Copy link
Member Author

Choose a reason for hiding this comment

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

I have no Idea, I thought it would be best to take an optimistic approach and restrict it when its not working. If I'd restrict the platform to linux now, probably nobody would test it for other platforms.

I don't know what the policy here is though, and I'll of course update that if the policy is to only add the platforms that were actually tested.

@7c6f434c
Copy link
Member

7c6f434c commented Nov 6, 2017

Oh, thanks.

Re: dependencies — well, there are tests, and there is a mechanism of replacing a dependency with a system-wide one (when I tried to package 7.x that was complicated, and eventually I got stuck when I couldn't understand which test failures were real). Adding proper versions to Nixpkgs is the relatively easy part even without automation.

@timokau
Copy link
Member Author

timokau commented Nov 6, 2017

Yeah that's another reason why I stopped going down that path for now and decided on just shipping something working. I think automation would be needed though, otherwise we would need to basically duplicate all the packaging work sage is doing for every update and the package would just stay outdated.

@7c6f434c
Copy link
Member

7c6f434c commented Nov 6, 2017 via email

@7c6f434c
Copy link
Member

7c6f434c commented Nov 6, 2017 via email

@timokau
Copy link
Member Author

timokau commented Nov 6, 2017

They have tests, but they also patch a lot of their dependencies.

@7c6f434c
Copy link
Member

7c6f434c commented Nov 6, 2017

Some of their patches seem to be just related to how they organize the builds, though.

@7c6f434c 7c6f434c merged commit 6475fa9 into NixOS:master Nov 6, 2017
@timokau
Copy link
Member Author

timokau commented Nov 6, 2017

Maybe, I didn't look into the details.

I think the best approach would be to start with getting it working with all the exact same dependencies, just build by nix. From there its trivial to switch out different dependencies and test what is working.

@timokau
Copy link
Member Author

timokau commented Nov 6, 2017

I think we try to follow a reasonable estimation of success chances. I don't think there is a policy that is actually followed. But this is a huge heavy build (hopefully someone will find a way to make it smaller), and it is known to be brittle, so I would recommend restricting testing it on Darwin to people who know how to override meta.broken

My concern was that nobody would actually know that it was available and so nobody would test it.

But I'm fine with changing it, maybe someone with a darwin system will see the commit and test.

@timokau timokau deleted the sage-fix branch November 6, 2017 03:09
@7c6f434c
Copy link
Member

7c6f434c commented Nov 6, 2017

Well, in reality it does sometimes happen that people using macOS try to build packages declared Linux-only and then ask to change the platform set (or submit a PR with fixes, if some changes to the build are needed).

I think the Darwin part of Hydra is weaker than the Linux part, too; and without tests we won't even know if the builds are useful until someone with a Mac investigates.

@timokau
Copy link
Member Author

timokau commented Nov 6, 2017

Okay, you're probably right that would be an unnecessary high load on hydra.

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.

SageMath is broken and outdated Sage package needs old texlive
3 participants