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

aspino: fix build #36933

Merged
merged 1 commit into from Mar 17, 2018
Merged

aspino: fix build #36933

merged 1 commit into from Mar 17, 2018

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Mar 13, 2018 •
edited

Motivation for this change

Aspino patched libglucose for their own uses, however they currently
depend on glucose v4.0.
(see https://github.com/alviano/aspino/tree/e31c3b4e5791a454e6602439cb26bd98d23c4e78/patches)

The patches don't apply properly on glucose-4.1 anymore, furthermore
the new source directory caused the bootstrap.sh from aspino which
was supposed to apply the patches and recompile the setup to break.

Furthermore some minor changes to the derivation were introduced:

  • upgraded from 2016-01-31 to 2017-03-09
  • the name contains an -unstable- infix as upstream has no releases
  • instead of a patchPhase the postPatch hook will be used for
    substituteInPlace to keep advanced patching features from nixpkgs
    available.
  • patchShebangs will be called to avoid impurities because of the
    implicit reliance on /bin/sh
  • added myself as second maintainer to have more people available in
    case of any further breackage

See https://hydra.nixos.org/build/70688471/log
See ticket #36453

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.

Ma27
Ma27 commented Mar 13, 2018
View reviewed changes
pkgs/applications/science/logic/aspino/default.nix Outdated
let
glucose' = glucose.overrideAttrs (_: {
src = fetchurl {
url = "http://www.labri.fr/perso/lsimon/downloads/softwares/glucose-syrup.tgz";
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 copied this URL from the last glucose bump, however a versioned URL is preferable

Copy link
Member Author

Choose a reason for hiding this comment

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

unfortunately the Glucose homepage currently points to this URL as well for v4.0 (http://www.labri.fr/perso/lsimon/glucose/), furthermore http://www.labri.fr/perso/lsimon/downloads/softwares/glucose-syrup-4.0.tgz returns a 404 so I'm afraid that we have to keep this for now

@ryantm
Copy link
Member

ryantm commented Mar 15, 2018 •
edited

Typically the way people solve this is they add back in the old package as something like glucose_4_0 at the top level, then you can do {glucose = glucose_4_0;} in the callPackage for aspino.

@Ma27
Copy link
Member Author

Ma27 commented Mar 15, 2018

@ryantm I know about this pattern, however I’ve mainly observed this with frequently used libraries where several versions are under active development or maintenance.

The bump of libglucose was IMO the right decision as 4.0 is way outdated ATM. However if that approach is considered as general rule, I can fix the libglucose derivation accordingly:)

@ryantm
Copy link
Member

ryantm commented Mar 15, 2018

Okay, I'm not sure if it's a general rule. Hopefully someone with more experience can also weigh in.

@vcunat
Copy link
Member

vcunat commented Mar 17, 2018

/cc maintainer: @gebner.

gebner
gebner requested changes Mar 17, 2018
View reviewed changes
pkgs/applications/science/logic/aspino/default.nix Outdated
{ stdenv, fetchurl, fetchFromGitHub, zlib, boost, glucose }:

let
glucose' = glucose.overrideAttrs (_: {
Copy link
Member

Choose a reason for hiding this comment

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

We don't even need to build glucose here, we just need the sources. Please remove the override and just use glucoseSrc = fetchurl ....

It's unfortunate that upstream did not manage to use versioned tarballs, but there's nothing we can do about that.

Copy link
Member Author

Choose a reason for hiding this comment

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

ahh, you're absolutely right, will fix this.

It's unfortunate that upstream did not manage to use versioned tarballs, but there's nothing we can do about that.

this is one of the reasons why I added myself as maintainer as well, so in case of any breackage in the future I can help out :)

@gebner
Copy link
Member

gebner commented Mar 17, 2018

Since we just need the glucose sources here (and for a specific version nevertheless!), I think that we don't need a separate glucose package.

Everything else looks good to me.

@Ma27
aspino: fix build
29bc0a0 
Aspino patched `libglucose` for their own uses, however they currently
depend on glucose v4.0.
(see https://github.com/alviano/aspino/tree/e31c3b4e5791a454e6602439cb26bd98d23c4e78/patches)

The patches don't apply properly on `glucose-4.1` anymore, furthermore
the new source directory caused the `bootstrap.sh` from `aspino` which
was supposed to apply the patches and recompile the setup to break.

Furthermore some minor changes to the derivation were introduced:

- upgraded from `2016-01-31` to `2017-03-09`
- the name contains an `-unstable-` infix as upstream has no releases
- instead of a `patchPhase` the `postPatch` hook will be used for
  `substituteInPlace` to keep advanced patching features from `nixpkgs`
  available.
- `patchShebangs` will be called to avoid impurities because of the
  implicit reliance on `/bin/sh`
- added myself as second maintainer to have more people available in
  case of any further breackage

See https://hydra.nixos.org/build/70688471/log
See ticket NixOS#36453
@gebner
Copy link
Member

gebner commented Mar 17, 2018

@GrahamcOfBorg build aspino

@GrahamcOfBorg
Copy link

Failure on aarch64-linux (full log)

Attempted: aspino

Partial log (click to expand)

src/main.cc:42:59: error: '_FPU_DOUBLE' was not declared in this scope
     _FPU_GETCW(oldcw); newcw = (oldcw & ~_FPU_EXTENDED) | _FPU_DOUBLE; _FPU_SETCW(newcw);
                                                           ^~~~~~~~~~~
src/main.cc:42:59: note: suggested alternative: '_GNU_SOURCE'
     _FPU_GETCW(oldcw); newcw = (oldcw & ~_FPU_EXTENDED) | _FPU_DOUBLE; _FPU_SETCW(newcw);
                                                           ^~~~~~~~~~~
                                                           _GNU_SOURCE
make: *** [Makefile:58: build/release/main.o] Error 1
builder for '/nix/store/anhs26af95zwaajinxi9197by4mffvy8-aspino-unstable-2017-03-09.drv' failed with exit code 2
�[31;1merror:�[0m build of '/nix/store/anhs26af95zwaajinxi9197by4mffvy8-aspino-unstable-2017-03-09.drv' failed

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: aspino

Partial log (click to expand)

shrinking /nix/store/s99gz35dkn0f513rbjk98qcn6fvm34c0-aspino-unstable-2017-03-09/bin/aspino
shrinking /nix/store/s99gz35dkn0f513rbjk98qcn6fvm34c0-aspino-unstable-2017-03-09/bin/fairino-bs
shrinking /nix/store/s99gz35dkn0f513rbjk98qcn6fvm34c0-aspino-unstable-2017-03-09/bin/maxino-2015-kdyn
shrinking /nix/store/s99gz35dkn0f513rbjk98qcn6fvm34c0-aspino-unstable-2017-03-09/bin/maxino-2015-k16
shrinking /nix/store/s99gz35dkn0f513rbjk98qcn6fvm34c0-aspino-unstable-2017-03-09/bin/fairino-ls
strip is /nix/store/fzcs0fn6bb04m82frhlb78nc03ny3w55-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/s99gz35dkn0f513rbjk98qcn6fvm34c0-aspino-unstable-2017-03-09/bin
patching script interpreter paths in /nix/store/s99gz35dkn0f513rbjk98qcn6fvm34c0-aspino-unstable-2017-03-09
checking for references to /build in /nix/store/s99gz35dkn0f513rbjk98qcn6fvm34c0-aspino-unstable-2017-03-09...
/nix/store/s99gz35dkn0f513rbjk98qcn6fvm34c0-aspino-unstable-2017-03-09

@gebner gebner merged commit fe7d82e into NixOS:master Mar 17, 2018
@Ma27 Ma27 deleted the fix-aspino branch March 17, 2018 12:06
@gebner
Copy link
Member

gebner commented Mar 17, 2018

Cherry-picked to 18.03 as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gebner gebner gebner approved these changes

Assignees
No one assigned
Labels
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
5 participants
@Ma27 @ryantm @vcunat @gebner @GrahamcOfBorg