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.

Sign up for GitHub

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

Jump to bottom

envoy 1.19.1 #83921

Closed
wants to merge 3 commits into from
Closed

envoy 1.19.1 #83921

wants to merge 3 commits into from

Conversation

codefromthecrypt
Copy link
Contributor

@BrewTestBot BrewTestBot added go Go use is a significant feature of the PR or issue no ARM bottle Formula has no ARM bottle labels Aug 25, 2021
@codefromthecrypt
Copy link
Contributor Author

@chenrui333 ps I switched the syntax to favor automation

codefromthecrypt
codefromthecrypt commented Aug 25, 2021
View reviewed changes
Formula/envoy.rb Outdated Show resolved Hide resolved
@codefromthecrypt
Copy link
Contributor Author

Also I looked at v1.19.1 doesn't include the patch even if the PR was merged. I don't know why but envoy usually doesn't cherry-pick so probably v1.20.0 will be the first version we can kill the patch with

@codefromthecrypt
Copy link
Contributor Author

what an odd failure.. I wonder if it is transient (on ubuntu)

[0 / 4] [Prepa] BazelWorkspaceStatusAction stable-status.txt ... (2 actions, 0 running)
ERROR: <builtin>: BazelWorkspaceStatusAction stable-status.txt failed: Failed to determine workspace status: Process exited with status 1
fatal: not a git repository (or any of the parent directories): .git
Target //source/exe:envoy-static failed to build
INFO: Elapsed time: 61.624s, Critical Path: 0.07s

@codefromthecrypt
Copy link
Contributor Author

force pushed just in case the problem is transient

@codefromthecrypt
Copy link
Contributor Author

no luck on this being transient. Can I call in an escalation? maybe @keith to help try to get 1.19.1 building? seems 1.18.4 worked alright on linux, too. #83929

This was referenced Aug 25, 2021
Open
Closed
@cho-m
Copy link
Member

cho-m commented Aug 25, 2021

The error seems to that envoy build is checking for Git information; however, GitHub generated source tarballs usually exclude this.

I believe the following script is run via Bazel's --workspace_status_command: https://github.com/envoyproxy/envoy/blob/main/bazel/get_workspace_status

The easiest way to fix would be to switch back to direct Git clone rather than source tarball.

@cho-m cho-m added the CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. label Aug 25, 2021
@codefromthecrypt
Copy link
Contributor Author

thanks for the analysis @cho-m so we can switch back I will make a comment basically we can't use automatic update scripts until this is fixed I think.

@codefromthecrypt codefromthecrypt mentioned this pull request Aug 25, 2021
Merged
@codefromthecrypt
Copy link
Contributor Author

@chenrui333 @carlocab would you mind doing the honors? here and #83929? I will raise an issue upstream to see if the project can build without requiring git (so that bump automation can work) out-of-band

@codefromthecrypt
Copy link
Contributor Author

@cho-m thanks for the explanation. I put in a comment so that someone doesn't "fix" this and break it!

@codefromthecrypt codefromthecrypt mentioned this pull request Aug 25, 2021
Closed
@codefromthecrypt
Copy link
Contributor Author

follow-up is here envoyproxy/envoy#17859

SMillerDev
SMillerDev reviewed Aug 26, 2021
View reviewed changes
Formula/envoy.rb
Comment on lines +4 to +5
# TODO: We can't use the tar.gz archive URL, which means bump-homebrew-formula-action doesn't work.
# See https://github.com/envoyproxy/envoy/issues/17859
Copy link
Member

Choose a reason for hiding this comment

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

You can use https://github.com/marketplace/actions/homebrew-bump-formula instead, but this doesn't need to be documented in the formula.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack for next time, thanks!

carlocab
carlocab approved these changes Aug 26, 2021
View reviewed changes
Copy link
Member

@carlocab carlocab left a comment

Choose a reason for hiding this comment

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

We can nix the comment in a separate syntax-only PR, as this CI run is pretty expensive.

@BrewTestBot
Copy link
Member

🤖 A scheduled task has triggered a merge.

@codefromthecrypt codefromthecrypt deleted the update-envoy.rb-1629863648 branch August 26, 2021 20:52
@github-actions github-actions bot added the outdated PR was locked due to age label Sep 26, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@SMillerDev SMillerDev SMillerDev left review comments

@carlocab carlocab carlocab approved these changes

Assignees
No one assigned
Labels
CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. go Go use is a significant feature of the PR or issue no ARM bottle Formula has no ARM bottle outdated PR was locked due to age
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@codefromthecrypt @cho-m @BrewTestBot @SMillerDev @carlocab