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

ocamlPackages.elpi: use release tarball #343266

Merged
merged 2 commits into from
Sep 23, 2024
Merged

Conversation

vbgl
Copy link
Contributor

@vbgl vbgl commented Sep 20, 2024

Description of changes

Released tarballs include correct version information.

Tests: coq-community/coq-nix-toolbox#266

cc @gares @proux01

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.11 Release Notes (or backporting 23.11 and 24.05 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.

}:

let p5 = camlp5; in
let camlp5 = p5.override { legacy = true; }; in

let fetched = coqPackages.metaFetch ({
Copy link
Contributor

Choose a reason for hiding this comment

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

@vbgl removing this will break the ability to conveniently overlay ocamlPackages.elpi in various CIs to anything else than released versions, which might be pretty unconvenient. Although I'm not sure how often we use that feature.
Cc @CohenCyril @gares ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. So my question is: can we use metaFetch to download the released artifacts when available?

Copy link
Contributor

Choose a reason for hiding this comment

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

I use that feature via the coq nix toolbox.
But I also need elpi to have a version number (for conditional compilation).

I hope both requirements are not in conflict.

Copy link
Contributor

Choose a reason for hiding this comment

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

can we use metaFetch to download the released artifacts when available?

I don't know, but if not, maybe an option can be added?

Copy link
Contributor

@CohenCyril CohenCyril Sep 20, 2024

Choose a reason for hiding this comment

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

Right. So my question is: can we use metaFetch to download the released artifacts when available?

Sure, feel free to amend the default-fetcher here:
https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/coq/meta-fetch/default.nix
You can add an extra "artifact" argument (false by default?) to change the url in the github case here:

{ cond = pr != null && (match "^github.*" domain) != null;
out = "https://api.${domain}/repos/${owner}/${repo}/${fmt}/pull/${head pr}/head"; }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I use fetchurl when you probably use fetchzip?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ... sure, but I thought the uncompressed archives were compared... why is the result different?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fetchurl does not decompress: it computes the hash of the downloaded file.

Copy link
Contributor

Choose a reason for hiding this comment

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

done: da329e4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Cherry-picked to this branch.

@vbgl vbgl merged commit 94d0d22 into NixOS:master Sep 23, 2024
24 checks passed
@vbgl vbgl deleted the ocaml-elpi-clean branch September 23, 2024 07:52
vbgl added a commit to vbgl/coq-nix-toolbox that referenced this pull request Sep 23, 2024
vbgl added a commit to coq-community/coq-nix-toolbox that referenced this pull request Sep 23, 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.

6 participants