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

chore: Change the format for sha512 sum for releases #25577

Merged
merged 1 commit into from
Oct 19, 2023

Conversation

sebastianliebscher
Copy link
Contributor

@sebastianliebscher sebastianliebscher commented Oct 9, 2023

SUMMARY

The current release.tar.gz.sha512 file format does not allow an easy comparison of the SHA sum. See #25333

Resolves #25333

Mirrors apache/airflow#12867

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@sfirke
Copy link
Member

sfirke commented Oct 17, 2023

Thanks for this @sebastianliebscher ! I don't understand the original if/else logic, can you explain what is changed by removing this outside of it and placing it at the end?

@rusackas
Copy link
Member

rusackas commented Oct 17, 2023

As someone who tends to validate releases, I appreciate this. That said, it's a convenience more than a necessity, so I haven't been losing sleep over it. What I'm not clear on is the if/else statement, and if skipping that would cause unforeseen issues. I'm very tempted to approve otherwise...

else
# The GPG key name to use
GPG_LOCAL_USER="${2}"
gpg --local-user "${GPG_LOCAL_USER}" --armor --output "${NAME}".asc --detach-sig "${NAME}"
gpg --local-user "${GPG_LOCAL_USER}" --print-md SHA512 "${NAME}" > "${NAME}".sha512
fi
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This checks if a GPG user-id was specified to use gpg with --local-user for signing/decrypting. Generating SHA sums (--print-md SHA512) is not affected by specifying this --local-user option.

So it is the same as

if [ -z "${2}" ]; then
  gpg --armor --output "${NAME}".asc --detach-sig "${NAME}"
else
  # The GPG key name to use
  GPG_LOCAL_USER="${2}"
  gpg --local-user "${GPG_LOCAL_USER}" --armor --output "${NAME}".asc --detach-sig "${NAME}"
fi
gpg --print-md SHA512 "${NAME}" > "${NAME}".sha512

which I then replaced with shasum:

if [ -z "${2}" ]; then
  gpg --armor --output "${NAME}".asc --detach-sig "${NAME}"
else
  # The GPG key name to use
  GPG_LOCAL_USER="${2}"
  gpg --local-user "${GPG_LOCAL_USER}" --armor --output "${NAME}".asc --detach-sig "${NAME}"
fi
shasum -a 512 "${NAME}" > "${NAME}.sha512"

Copy link
Member

@michael-s-molina michael-s-molina 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 for the improvement @sebastianliebscher!

@michael-s-molina michael-s-molina merged commit 505678b into apache:master Oct 19, 2023
29 checks passed
@sebastianliebscher sebastianliebscher deleted the shasum_format branch October 21, 2023 14:00
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 3.1.0 labels Mar 8, 2024
sfirke pushed a commit to sfirke/superset that referenced this pull request Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/XS 🚢 3.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The checksums are difficult to validate on your official source releases
5 participants