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

Use docker environment to build the actors reproducibly #865

Closed

Conversation

ianconsolata
Copy link

@ianconsolata ianconsolata commented Nov 22, 2022

I didn't have permissions to push to the original branch in #634, so I made my own fork.

@ianconsolata ianconsolata changed the title Id/reproducible build Use docker environment to build the actors reproducibly Nov 22, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #865 (df2b7cf) into master (fdcf50b) will increase coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head df2b7cf differs from pull request most recent head 582587e. Consider uploading reports for the commit 582587e to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #865      +/-   ##
==========================================
+ Coverage   88.08%   88.10%   +0.02%     
==========================================
  Files          95       95              
  Lines       19619    19628       +9     
==========================================
+ Hits        17281    17294      +13     
+ Misses       2338     2334       -4     
Impacted Files Coverage Δ
runtime/src/test_utils.rs 83.00% <100.00%> (+0.19%) ⬆️
actors/verifreg/src/lib.rs 92.84% <0.00%> (+0.12%) ⬆️
actors/power/src/lib.rs 83.83% <0.00%> (+0.21%) ⬆️
actors/multisig/src/lib.rs 95.04% <0.00%> (+0.23%) ⬆️
actors/init/src/lib.rs 98.14% <0.00%> (+1.85%) ⬆️

@anorth
Copy link
Member

anorth commented Nov 22, 2022

Thanks for moving things into Makefile

Copy link
Contributor

@ZenGround0 ZenGround0 left a comment

Choose a reason for hiding this comment

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

Out of an abundance of caution we shouldn't merge this until a few days after the upgrade has settled in case we're fighting fires and quickly releasing things. Once that window has passed -- say this friday this looks good to merge.

@ianconsolata ianconsolata force-pushed the id/reproducible-build branch from 582587e to ff5db68 Compare December 6, 2022 11:13
@ianconsolata
Copy link
Author

ianconsolata commented Dec 6, 2022

Just rebased onto filecoin-project/master to bring this PR up to date with recent changes. I think the upgrade from last week has settled enough we should be good to merge this now?

@rjan90
Copy link
Contributor

rjan90 commented Jan 8, 2025

Hey! 👋

As part of our cleanup to kick off the year, I'm reviewing all open non-draft pull requests. Could you please do one of the following for your PR?

1. Close it: If it's no longer needed.
2. Mark as Draft: If it needs more work, and add the next steps.
3. Ready for Review: If it's good to go, let me know, and I'll assign a reviewer.

If there's no response in a week, I'll assume it's option 1 and close the PR. If you have any questions, just let me know.

Thanks for your help in keeping things organized, and I appreciate your contributions!

@ianconsolata
Copy link
Author

Hi @rjan90 this was ready for review two years ago, not sure if it's still needed or what needs to be updated to merge it

@ianconsolata
Copy link
Author

It's probably far enough out of date at this point it should be closed, but I don't know if the original issue this PR and the previous PR were trying to address has been fixed some other way.

@rjan90
Copy link
Contributor

rjan90 commented Jan 15, 2025

Yeah, I´m uncertain if it is still needed. But I agree with you that it is probably best to:

  1. Close this PR.
  2. Send a ping in Reproducable Build #171, saying that this PR which tried to resolve it has been closed due to missing reviews/progress, and if there is a want to bring this back to life this PR is worthy a peek.
    • If the issue has already been resolved/fixed, the original issue should be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ☑️ Done (Archive)
Development

Successfully merging this pull request may close these issues.

6 participants