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

compcert: fix source hash #57848

Merged
merged 1 commit into from Mar 20, 2019
Merged

compcert: fix source hash #57848

merged 1 commit into from Mar 20, 2019

Conversation

vbgl
Copy link
Contributor

@vbgl vbgl commented Mar 18, 2019

Motivation for this change

Current package is broken (the contents of the source tarball have been updated).

Things done
  • 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/)
  • 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.

@@ -16,15 +16,14 @@ stdenv.mkDerivation rec {

src = fetchurl {
url = "http://compcert.inria.fr/release/${name}.tgz";
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be more stable https://github.com/AbsInt/CompCert

Hopefully the archives don't defer too much.

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 believe that both sources are tightly coupled. How could one be more stable than the other?

Copy link
Member

Choose a reason for hiding this comment

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

@worldofpeace is saying that by using the Git repository tags to fetch rather than the inria.fr server, there will probably be no force pushes to master which would retroactively invalidate hashes. Which I'd agree with, and probably would make it more stable in the future if this repeatedly happens -- but in this case we don't even know what changed, unfortunately (there are no commits to CompCert since the 3.5 tag was posted, which is odd...)

Copy link
Contributor

Choose a reason for hiding this comment

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

It's also https

@Mic92
Copy link
Member

Mic92 commented Mar 19, 2019

@xavierleroy any idea why the hashes changes?

@xavierleroy
Copy link

Yes, I forgot to update a file, so I re-did the release. The bad release was out for less than 24 hours and hadn't been announced, so I took the liberty of fixing it rather than do another release. I didn't know you packagers worked so fast :-)

Release tarballs can be downloaded from caml.inria.fr or from github.com, but I would use github.com as more reliable overall.

Finally, there was no commit to master since 3.5 release, indeed. Nothing to worry about, just some work that is going on on branches and that will be merged soon.

Hope this answers all your questions.

@vbgl
Copy link
Contributor Author

vbgl commented Mar 20, 2019

Thanks for the explanation. I’ve thus updated the package to fetch the sources from the github tag.

@Mic92
Copy link
Member

Mic92 commented Mar 20, 2019

@GrahamcOfBorg build compcert

Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

Builds successfully on NixOS.

@worldofpeace worldofpeace merged commit c1d65d6 into NixOS:master Mar 20, 2019
@vbgl vbgl deleted the compcert-3.5-fix branch March 21, 2019 09:14
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

6 participants