add error log of download formula.json
#13997
Conversation
Library/Homebrew/cmd/update.sh
Outdated
| exit_code=$? | ||
| if [[ ${exit_code} -ne 0 ]] |
There was a problem hiding this comment.
| exit_code=$? | |
| if [[ ${exit_code} -ne 0 ]] | |
| curl_exit_code=$? | |
| if [[ ${curl_exit_code} -ne 0 ]] |
| @@ -761,8 +761,11 @@ EOS | |||
| --time-cond "${HOMEBREW_CACHE}/api/formula.json" \ | |||
| --user-agent "${HOMEBREW_USER_AGENT_CURL}" \ | |||
| "https://formulae.brew.sh/api/formula.json" | |||
| # TODO: we probably want to print an error if this fails. | |||
| # TODO: set HOMEBREW_UPDATED or HOMEBREW_UPDATE_FAILED | |||
There was a problem hiding this comment.
Would be good to set HOMEBREW_UPDATED="1" if the file checksum changes?
Library/Homebrew/cmd/update.sh
Outdated
| @@ -754,16 +754,26 @@ EOS | |||
| if [[ -n "${HOMEBREW_INSTALL_FROM_API}" ]] | |||
| then | |||
| mkdir -p "${HOMEBREW_CACHE}/api" | |||
| if [[ -f "${HOMEBREW_CACHE}/api/formula.json" ]] | |||
| then | |||
| INITIAL_JSON_SHASUM="$(shasum -a 256 "${HOMEBREW_CACHE}"/api/formula.json)" | |||
There was a problem hiding this comment.
See vendor-install.sh. Can't rely on shasum being installed, unfortunately. Could consider just using e.g. the file size or something instead? This isn't for security so false-negatives don't matter too much.
There was a problem hiding this comment.
Thanks for the review; I will put the ruby script to check shasum256 instead of shasum.
There was a problem hiding this comment.
No, I think you want to do checksums instead of e.g. file sizes I think it's worth splitting out a shasum helper or something that we can use both here and in vendor-install.
There was a problem hiding this comment.
I will compare the file size by ls -s "${HOMEBREW_CACHE}"/api/formula.json)".
There was a problem hiding this comment.
@hyuraku actually the best bet would be wc -c "${HOMEBREW_CACHE}"/api/formula.json)"
Co-authored-by: Bo Anderson <mail@boanderson.me>
| CURRENT_JSON_BYTESIZE="$(wc -c "${HOMEBREW_CACHE}"/api/formula.json)" | ||
| if [[ "${INITIAL_JSON_BYTESIZE}" != "${CURRENT_JSON_BYTESIZE}" ]] | ||
| then | ||
| HOMEBREW_UPDATED="1" | ||
| fi |
There was a problem hiding this comment.
Thanks for this PR, @hyuraku! Looks great!
Sorry for the late comment. Would it make more sense to do this comparison based on the file update time? We pass --remote-time to curl and use this modification time anyway with --time-cond, so it seems reasonable to use that to see whether the file was updated. This removes the (unlikely) edge case where the pre and post files are the same thing
brew stylewith your changes locally?brew typecheckwith your changes locally?brew testswith your changes locally?repair for
We need to handle download errors of the formula.json and cask.json filesin #13794