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

oracle-instantclient: simplify a lot, use autoPatchelfHook #40900

Merged
merged 3 commits into from May 22, 2018

Conversation

flokli
Copy link
Contributor

@flokli flokli commented May 22, 2018

Motivation for this change

After fixing some things in oracle-instantclient for #40688, the expression still was very messy.

I did some refactoring of the build phase, and thanks to autoPatchelfHook we can get rid of all the manual patchelfing, turning the expression a lot cleaner.

As now everything is much more maintainable, I'm happy with adding myself as one :-)

Tested connecting to an oracle database through python3Packages.sqlalchemy and python3Packages.cx_oracle, plus executing the sqlplus binary.

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.

Copy link
Contributor

@xeji xeji left a comment

Choose a reason for hiding this comment

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

Nice, a lot shorter now.


nativeBuildInputs = [ rpmextract patchelf makeWrapper ];
unpackCmd = "${rpmextract}/bin/rpmextract $curSrc";
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add rpmextract to nativeBuildInputs since it is run during the build process.
Then it will be available in PATH and you can omit the prefix ${rpmextract}/bin here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

Split `buildCommand`, provide `unpackCmd` and add `installPhase`.

Use autoPatchelfHook, we can get rid of all the manual hacking around
with patchelf.

Use install to install to $out
we requireFile, so hydra can't build it anyways.
@xeji xeji merged commit 5093abc into NixOS:master May 22, 2018
@flokli flokli deleted the oracle-fixes branch May 22, 2018 09:37
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

3 participants