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

feat: support user defined version index (incl. pre-release) #290

Merged
merged 18 commits into from
May 4, 2024

Conversation

aherrmann
Copy link
Owner

@aherrmann aherrmann commented May 4, 2024

Depends on #291

Closes #76

To register a nightly toolchain a user must add a version index JSON file (as a available on https://ziglang.org/download/index.json) to their project, register it using the zig.index(file = ...) tag, and then register the version using zig.toolchain(zig_version = ...).

This also provides support for the broader interpretation of #72.

  • Add the zig.index tag.
  • Add doc string for the zig.toolchain tag
  • handle_tags --> handle_toolchain_tags
  • refactor: handle_toolchain_tags takes module list
  • docs: update generated API docs
  • feat: Support user defined Zig version index files
  • Support JSON parsing error reporting
  • Test Zig version index parsing
  • Test JSON parsing failure cases
  • Parse nightly version
  • Test full index format parsing
  • Filter out unknown platforms
  • Test for bad version numbers
  • Validate version numbers
  • Test version spec merging
  • Integration test for extra versions index
  • adjust test size
  • docs: specify default as latest known version

@aherrmann aherrmann changed the base branch from main to bazel-version May 4, 2024 15:49
Base automatically changed from bazel-version to main May 4, 2024 16:22
Copy link
Contributor

@Titousensei Titousensei left a comment

Choose a reason for hiding this comment

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

A couple of nit comments, otherwise looks good.

for platform, info in platforms.items():
if type(info) != "dict" or not platform in PLATFORMS:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this can ever be true: type() returns a class, never a string.

Copy link
Owner Author

Choose a reason for hiding this comment

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

This is Starlark, not Python. type returns a string in this case https://bazel.build/rules/lib/globals/all#type

data = json.decode(json_string, default = None)

if data == None:
return "Invalid JSON format in Zig SDK version index.", None
Copy link
Contributor

Choose a reason for hiding this comment

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

please consider raising a custom exception instead of returning an error string: that way you don't have to repeat the error strings in tests, and the control flow is more explicit with try-except.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Unfortunately there is no try catch in Starlark. This code uses something akin to go style error returns to make error cases testable.

@aherrmann aherrmann merged commit 8593257 into main May 4, 2024
61 of 81 checks passed
@aherrmann aherrmann deleted the user-versions branch May 4, 2024 17:32
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.

Support Zig nightly releases.
2 participants