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

Jump to bottom

giac: 1.4.9 -> 1.5.0 #51786

Merged
merged 5 commits into from Dec 10, 2018
Merged

giac: 1.4.9 -> 1.5.0 #51786

merged 5 commits into from Dec 10, 2018

Conversation

symphorien
Copy link
Member

Motivation for this change

update

this also fixes a bug where the xcas ui (notably menus) were broken because of split outputs. Apparently xcas uses its documentation to populate some ui elements.
I added a few optional dependencies.

Things done
  • built sage with it. Patch sent upstream. cc @timokau
  • modiffied nauty to also install its libraries. cc @7c6f434c as maintainer
  • 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/) only xcas
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

7c6f434c
7c6f434c approved these changes Dec 9, 2018
View reviewed changes
Copy link
Member

@7c6f434c 7c6f434c left a comment

Choose a reason for hiding this comment

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

Looks fine to me. As giac is only there for sage (and I added it when trying to bump Sage), I would prefer to defer to @timokau — so I will not merge myself earlier than after a couple of says.

timokau
timokau requested changes Dec 9, 2018
View reviewed changes
Copy link
Member

@timokau timokau left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of this! Giac can be a bit of a pain to deal with. Looks good to me, just two minor nitpicks.

pkgs/applications/science/math/nauty/default.nix Outdated

cp $(find . -type f -perm -111 \! -name '*.*' ) "$out/bin"
cp $(find . -type f -perm -111 \! -name '*.*' \! -name configure) "$out/bin"
Copy link
Member

Choose a reason for hiding this comment

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

Would be better to use find -exec cp '{}' target \; instead

pkgs/applications/science/math/nauty/default.nix Outdated

cp *.h $dev/include/nauty
for i in *.a; do
cp $i $dev/lib/lib$i;
Copy link
Member

Choose a reason for hiding this comment

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

Should be quoted

@timokau
Copy link
Member

timokau commented Dec 9, 2018

Also even smaller nitpicks (not necessary to fix those, just my preferences): I think the fixup commit should be merged with the update commit. And if the new dependencies are really optional, it would be nice to create a flag (default enable) for them.

@symphorien
Copy link
Member Author

Comments addressed.
About optional dependencies, you just have to override them to null if you want to get rid of them.
The configure script will detect their absence and do without, even if --enable-foo is present.

@timokau
Copy link
Member

timokau commented Dec 10, 2018

Great! I don't think just null'ing the dependencies is a good option, but as I said thats not a blocker. Thanks again!

@timokau timokau merged commit f0ea741 into NixOS:master Dec 10, 2018
@symphorien symphorien deleted the giac-1.5 branch May 18, 2019 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@7c6f434c 7c6f434c 7c6f434c approved these changes

@timokau timokau timokau approved these changes

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@symphorien @timokau @7c6f434c @GrahamcOfBorg