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

Add script to update magic numbers #108

Merged
merged 2 commits into from
Oct 16, 2024
Merged

Conversation

liam923
Copy link
Contributor

@liam923 liam923 commented Oct 16, 2024

Add a new script that will bump the magic numbers.

Running this script will:

  1. Verify that the git workspace is clean
  2. Update the magic numbers in src/ocaml/utils/config.ml
  3. Add a new entry in src/ocaml/typing/magic_numbers.ml

A test run of this script can be done by:

  1. Find and replacing 551 with 552 in upstream/ocaml_flambda/configure.
  2. Commit the above changes.
  3. Running ./update-magic-numbers.sh 5.2.0minus-2

I've performed a few test runs of this script, where:

  • All magic numbers changed
  • Some magic numbers changed
  • No magic numbers changed
  • The git workspace is unclean
  • There was already an entry in src/ocaml/typing/magic_numbers.ml
  • The magic numbers in upstream/ocaml_flambda/configure are not the expected format
  • A new magic number has been added to src/ocaml/utils/config.ml that the script doesn't know about

@liam923 liam923 requested a review from ncik-roberts October 16, 2024 16:33
let ast_intf_magic_number = "Caml1999N551"
let cmt_magic_number = "Caml1999T551"
let cms_magic_number = "Caml1999S551"
let index_magic_number = "Merl2023I501"

let interface_suffix = ref ".mli"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The changes in this file are meant to make the script a bit simpler - there's no material changes

Copy link
Contributor

@ncik-roberts ncik-roberts left a comment

Choose a reason for hiding this comment

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

Running the script produces a diff for me:

-let index_magic_number = "Merl2023I501"
+let index_magic_number = "Merl2023I551"

Copy link
Contributor

@ncik-roberts ncik-roberts left a comment

Choose a reason for hiding this comment

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

That change is expected, according to Liam.

@liam923 liam923 merged commit 71d4117 into main Oct 16, 2024
1 of 2 checks passed
@liam923 liam923 deleted the script-to-bump-magic-numbers branch October 16, 2024 17:58
dkalinichenko-js pushed a commit that referenced this pull request Nov 8, 2024
* Add script to update magic numbers

* Fix CI failure
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.

2 participants