Skip to content

Follow up from cron-daily-fuzz PR#684

Merged
apoelstra merged 2 commits intorust-bitcoin:masterfrom
tcharding:05-03-follow-up-cron-fuzz
May 14, 2024
Merged

Follow up from cron-daily-fuzz PR#684
apoelstra merged 2 commits intorust-bitcoin:masterfrom
tcharding:05-03-follow-up-cron-fuzz

Conversation

@tcharding
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member Author

tcharding commented May 7, 2024

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

@apoelstra
Copy link
Copy Markdown
Member

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

@tcharding
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

tcharding added 2 commits May 14, 2024 09:03
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 tcharding force-pushed the 05-03-follow-up-cron-fuzz branch from a86c0ee to bd1df30 Compare May 13, 2024 23:03
@tcharding
Copy link
Copy Markdown
Member Author

Ready to go!

Copy link
Copy Markdown
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
@tcharding tcharding deleted the 05-03-follow-up-cron-fuzz branch May 16, 2024 22:01
heap-coder added a commit to heap-coder/rust-miniscript that referenced this pull request Sep 27, 2025
…z PR

bd1df30a7944133374c4696f652515a8cdd1fba2 Sort fuzz files when finding (Tobin C. Harding)
73d81e5f145f8ff8c107602a337eb7c0d31dec11 Follow up from cron-daily-fuzz PR (Tobin C. Harding)

Pull request description:

  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).

ACKs for top commit:
  apoelstra:
    ACK bd1df30a7944133374c4696f652515a8cdd1fba2

Tree-SHA512: 2af4c030398a79d1de2b2b5d49a399806e21681e5783d97bc139deb6004fed6b34461fb3547070a767d6fff1f76421c775a0a7d1ddc1de05dc50a3d08682618d
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.

3 participants