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

Follow up from cron-daily-fuzz PR #684

Merged
merged 2 commits into from
May 14, 2024

Conversation

tcharding
Copy link
Member

I royally botched up PR #683 which changed fuzzing to run daily, fix it up by doing:

  • Use the correct yaml file name in generate-files.sh.
  • Actually run generate-files.sh to generate the yaml file.
  • Fix the time UTC comment (the other times were correctly set).

@tcharding tcharding mentioned this pull request May 2, 2024
sanket1729
sanket1729 previously approved these changes May 3, 2024
Copy link
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

utACK 1f4b007

@apoelstra
Copy link
Member

In 1f4b007:

I get a different sort order than you for the fuzztests. I think we need to set the locale.

@apoelstra
Copy link
Member

Actually we currently don't sort these at all.. we just run find and take the raw output. We should sort them.

@tcharding
Copy link
Member Author

tcharding commented May 7, 2024

Done as an additional patch and added rust-bitcoin/rust-bitcoin-maintainer-tools#5

@apoelstra
Copy link
Member

Guess we need to pin the nightly compiler here too :(

@tcharding
Copy link
Member Author

tcharding commented May 7, 2024

I was trying to get away from doing that in ever repo, I am not at all keen to be manually merging an "update msrv" patch on every repo we have twice a week, that is a job for a robot, or just kill me now. I mean, I can write a script that just does it but then we may as well have bot run the script. Its literally just, update msrv, if CI is green, merge it.

@apoelstra
Copy link
Member

@tcharding As a matter of policy, I don't want bots to merge things unfortunately. But I see your point. This is entirely mechanical and pretty annoying to do.

Maybe if we made the update be every 2 weeks or something?

@tcharding
Copy link
Member Author

I can handle once a week, I realised also that the first repo is the annoying one (requires installing the new toolchain, which is slow). The additional repos should be less painful.

@apoelstra
Copy link
Member

@tcharding in the "usual" case that CI passes, you don't need to install the toolchain.

But yes, when things break, at least you only need to do it once..

@tcharding
Copy link
Member Author

Huh? As soon as it merges everyone needs to install the new toolchain to be able to build with nightly (at least if they use the justfile and/or want to run the same nightly that CI will run).

@apoelstra
Copy link
Member

Oh, interesting. I did not realize this. In my local CI I am always using the latest nightly (which does mean that the first job of the day is a bit slow) and on the CLI I have just been using whatever nightly I happen to have laying around, and only updating it when things change.

@tcharding
Copy link
Member Author

tcharding commented May 10, 2024

Ah of course, I can just add rustup update to the script I run every morning when I first hit the keyboard.

I royally botched up PR rust-bitcoin#683 which changed fuzzing to run daily, fix it
up by doing:

- Use the correct yaml file name in `generate-files.sh`.
- Actually run `generate-files.sh` to generate the yaml file.
- Fix the time UTC comment (the other times were correctly set).
Currently we don't sort the output of fuzz file names when finding them,
this can lead to different output from `generate-files.sh` on different
machines.

Set the locale first because it effects sort order then sort the output
of `find`.
@tcharding
Copy link
Member Author

Ready to go!

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK bd1df30

@apoelstra apoelstra merged commit 351ffa8 into rust-bitcoin:master May 14, 2024
7 checks passed
@tcharding tcharding deleted the 05-03-follow-up-cron-fuzz branch May 16, 2024 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants