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

tools, github: Add current SOURCE_VERSION writer #23577

Merged
merged 9 commits into from
Oct 21, 2022

Conversation

dio
Copy link
Member

@dio dio commented Oct 20, 2022

Commit Message: This adds a script to produce the SOURCE_VERSION file which is useful to build envoy in a non-git (from an extracted release tarball) directory. See: bazel/get_workspace_status for more information.

Relevant: #2181.

Additional Description: This is useful to have a way to produce SOURCE_VERSION, for example, to solve: https://github.com/Homebrew/homebrew-core/blob/923bb7a622b86876a662d1ed4a7ac51ccb2befcf/Formula/envoy.rb#L4. So downstream builder can consume release tarballs to build envoy without adding custom scripts to create SOURCE_VERSION.
Risk Level: Low, since this is an optional tool.
Testing: Run the script in git and non-git environments.
Docs Changes: Added.
Release Notes: N/A
Platform-Specific Features: N/A

This adds a script to produce SOURCE_VERSION which is useful to build
envoy in a non-git (from an extracted release tarball) directory.

Signed-off-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Dhi Aurrahman <[email protected]>
@repokitteh-read-only
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #23577 was opened by dio.

see: more, trace.

@dio dio changed the title tools, github: Add current source version writer tools, github: Add current SOURCE_VERSION writer Oct 20, 2022
@dio dio marked this pull request as ready for review October 20, 2022 03:20
Signed-off-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Dhi Aurrahman <[email protected]>
@wbpcode
Copy link
Member

wbpcode commented Oct 20, 2022

/assign @phlax

Copy link
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

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

looks good @dio

im wondering if we should wrap the code in a main function and use a if __name__ == "__main__": block to trigger it - it just means it wont run just from being imported

im also wondering if we need to add some info somewhere to say to run this before running any bazel tasks (for a tarball build)

Signed-off-by: Dhi Aurrahman <[email protected]>
@dio
Copy link
Member Author

dio commented Oct 20, 2022

Thanks for the review @phlax. I added some lines in bazel/README.md. Please let me know if that makes sense (presented as a subsection, under the Production environments section, since probably that is what people want to do with release tarballs).

Signed-off-by: Dhi Aurrahman <[email protected]>
Copy link
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

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

one python suggestion and then i think this should be good to land

tools/github/write_current_source_version.py Outdated Show resolved Hide resolved
Signed-off-by: Dhi Aurrahman <[email protected]>
Copy link
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @dio

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