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

buildHarePackage: init #294881

Closed
wants to merge 3 commits into from
Closed

Conversation

onemoresuza
Copy link
Contributor

@onemoresuza onemoresuza commented Mar 11, 2024

Description of changes

  • buildHarePackage: init function
  • haredo: make use of buildHarePackage
  • hareThirdParty.hare-toml: make use of buildHarePackage

The buildHarePackage gets rid of boilerplate code for Hare packages:

  1. No need to set HARECACHE;
  2. No need to set PREFIX;
  3. No need to set -qRa<arch> hare flags.
  4. By setting -a${stdenv.hostPlatform.uname.processor}, the derivation needs
    to do nothing else for the package to cross-compile.

It, however, relies on the existence of the HAREFLAGS make or environment
variable. Thus, if not yet present on the project, the derivation must patch it.
This is behavior goes hand in hand with Hare's project structure described in its documentation.

Do note that HARECACHE, PREFIX and HAREFLAGS are set both as make flags and
as environment variables, so that it may be accessed by other build systems, e. g., haredo.

I've only changed haredo and hareThirdParty.hare-toml to make use of
buildHarePackage because I am their maintainer and we have with them both
examples of a Hare package: a program, the former, and a library, the latter.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/buildharepackage-looking-for-feedback/41189/1

@onemoresuza
Copy link
Contributor Author

Pinging Hare package maintainers for feedback: @starzation, @uninsane, @patwid and @auchter.

@uninsane
Copy link
Contributor

if the same goal here can be achieved via a setup hook, i would prefer that. mkDerivation + setup hooks is great as a package author because it represents a single workflow that works (and works consistently) for every occasion: the build{Lang}Package world is one in which i need to learn a new tool for every language despite that those distinct tools all aim to achieve the same thing as eachother in the end. it's kind of the opposite of abstraction.

it's also pretty tough to get a build*Package helper to work under all the same edge cases that stdenv.mkDerivation covers. package splicing and overriding are the usual problem makers, and then build*Package (finalAttrs: { ... }) is an idiom lots of these outright don't support.

anyway, i'm sure there's been more detailed discussion around what i just wrote before in nixpkgs, but i don't know where that would be :( the closest i could find is this discussion when zig was considering a buildZigPackage helper. might be worth pinging some of the more experienced maintainers from that thread on this if we're split.

@onemoresuza
Copy link
Contributor Author

if the same goal here can be achieved via a setup hook, i would prefer that. mkDerivation + setup hooks is great as a package author because it represents a single workflow that works (and works consistently) for every occasion: the build{Lang}Package world is one in which i need to learn a new tool for every language despite that those distinct tools all aim to achieve the same thing as eachother in the end. it's kind of the opposite of abstraction.

it's also pretty tough to get a build*Package helper to work under all the same edge cases that stdenv.mkDerivation covers. package splicing and overriding are the usual problem makers, and then build*Package (finalAttrs: { ... }) is an idiom lots of these outright don't support.

anyway, i'm sure there's been more detailed discussion around what i just wrote before in nixpkgs, but i don't know where that would be :( the closest i could find is this discussion when zig was considering a buildZigPackage helper. might be worth pinging some of the more experienced maintainers from that thread on this if we're split.

Good point, @uninsane. After reading it, it does make a lot more sense to have a setup hook, since Hare builds do not deviate that much from what stdenv.mkDerivation already does, and, as you said, we would avoid consistency problems by still using it instead of new build*Package.

I'll try to come up also with a hook and see what the other maintainers think about it. When we are ready to decide on which one, I'll rebase and remove the not chosen option.

@onemoresuza onemoresuza changed the title buildHarePackage: init function hare: init build framework Mar 11, 2024
@onemoresuza onemoresuza changed the title hare: init build framework buildHarePackage: init Mar 11, 2024
@onemoresuza
Copy link
Contributor Author

@uninsane, I've created a hare.hook PR. I've based its implementation on zig's. As of now, I'm unable to cross-compile, but both HAREFLAGS and HARECACHE are passed correctly both as an environment and as a make variable.

@onemoresuza
Copy link
Contributor Author

Moreover, for everybody on this PR, do you think it would be better to create an issue regarding both PRs? By doing so, I think it would be easier to keep track of them until the final decision is made.

@starzation
Copy link
Contributor

@onemoresuza Maybe it would be a good idea to create a new issue.

@onemoresuza
Copy link
Contributor Author

Closing this PR, since the decision was made for a hare.hook: #295155

@onemoresuza onemoresuza closed this Apr 9, 2024
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.

4 participants