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

Follow redirects for ktlint download #3543

Merged
merged 2 commits into from
Mar 4, 2024

Conversation

gmackall
Copy link
Member

@gmackall gmackall commented Mar 4, 2024

The github release page is redirecting, so follow the redirects.

Good news, the uploading is working and the cipd package is created: https://chrome-infra-packages.appspot.com/p/flutter/ktlint/linux-amd64

Bad news, the uploaded ktlint file is empty because the curl is failing because we aren't following the redirect.

Passing the -L to curl fixes this (I double checked that with -L passed locally, we download a working version of ktlint). Sorry, should have caught this in the original PR.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read the Flutter Style Guide recently, and have followed its advice.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@gmackall
Copy link
Member Author

gmackall commented Mar 4, 2024

Also verifiable in the presubmit step here that we are actually downloading something
https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8754355472971472689/+/u/build_package/stdout

curl -L https://github.com/pinterest/ktlint/releases/download/1.1.1/ktlint -o /b/s/w/ir/x/w/cocoon/cipd_packages/ktlint/tools/../build/ktlint
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0

 34 67.0M   34 23.3M    0     0  49.5M      0  0:00:01 --:--:--  0:00:01 49.5M
100 67.0M  100 67.0M    0     0   105M      0 --:--:-- --:--:-- --:--:--  266M

@gmackall gmackall marked this pull request as ready for review March 4, 2024 23:02
@gmackall gmackall requested a review from keyonghan March 4, 2024 23:02
@gmackall
Copy link
Member Author

gmackall commented Mar 4, 2024

Sorry for the spam, this will hopefully be the last of the cocoon ktlint prs 🙏

Copy link
Contributor

@keyonghan keyonghan left a comment

Choose a reason for hiding this comment

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

LGTM

@gmackall gmackall added the autosubmit Merge PR when tree becomes green via auto submit App. label Mar 4, 2024
@auto-submit auto-submit bot merged commit 7eae6dc into flutter:main Mar 4, 2024
4 checks passed
sealesj pushed a commit to sealesj/cocoon that referenced this pull request Mar 7, 2024
The github release page is redirecting, so follow the redirects. 

Good news, the uploading is working and the cipd package is created: https://chrome-infra-packages.appspot.com/p/flutter/ktlint/linux-amd64

Bad news, the uploaded ktlint file is empty because the curl is failing because we aren't following the redirect. 

Passing the `-L` to curl fixes this (I double checked that with `-L` passed locally, we download a working version of ktlint). Sorry, should have caught this in the original PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants