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

Promote libtommath to SPECS #3381

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Sumynwa
Copy link
Contributor

@Sumynwa Sumynwa commented Jul 15, 2022

Merge Checklist

All boxes should be checked before merging the PR (just tick any boxes which don't apply to this PR)

  • The toolchain has been rebuilt successfully (or no changes were made to it)
  • The toolchain/worker package manifests are up-to-date
  • Any updated packages successfully build (or no packages were changed)
  • Packages depending on static components modified in this PR (Golang, *-static subpackages, etc.) have had their Release tag incremented.
  • Package tests (%check section) have been verified with RUN_CHECK=y for existing SPEC files, or added to new SPEC files
  • All package sources are available
  • cgmanifest files are up-to-date and sorted (./cgmanifest.json, ./toolkit/tools/cgmanifest.json, ./toolkit/scripts/toolchain/cgmanifest.json, .github/workflows/cgmanifest.json)
  • LICENSE-MAP files are up-to-date (./SPECS/LICENSES-AND-NOTICES/data/licenses.json, ./SPECS/LICENSES-AND-NOTICES/LICENSES-MAP.md, ./SPECS/LICENSES-AND-NOTICES/LICENSE-EXCEPTIONS.PHOTON)
  • All source files have up-to-date hashes in the *.signatures.json files
  • sudo make go-tidy-all and sudo make go-test-coverage pass
  • Documentation has been updated to match any changes to the build system
  • Ready to merge

Summary

Promote libtommath to meet build dependency for package libtomcrypt.

Change Log
  • move from SPECS-EXTENDED to SPECS
  • add as build dependency for libtomcrypt
  • lint spec
Does this affect the toolchain?

NO

Associated issues
  • #xxxx
Links to CVEs
Test Methodology
  • Pipeline build id: 213527

@Sumynwa Sumynwa requested a review from a team as a code owner July 15, 2022 06:43
@ghost ghost added Packaging main PR Destined for main labels Jul 15, 2022
Copy link
Contributor

@oliviacrain oliviacrain left a comment

Choose a reason for hiding this comment

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

Please bump the release number of this package. We do this to ensure users with both the base and extended repos installed will pull the base repo version of the package.

That'll also allow me to actually comment on the file itself, too.

Copy link
Contributor

@PawelWMS PawelWMS left a comment

Choose a reason for hiding this comment

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

Similar comments as for #3374:

Specs from SPECS-EXTENDED rarely can be simply moved into SPECS without additional work as they have a lower bar in terms of test, spec structure, linting, etc.

Please make sure to now:

  • Lint the spec.
  • Make sure the ptests (inside the %check section) run successfully.
  • Remove all references to other distros (if any). In most cases we tend to keep the parts of the spec, which were enabled for Fedora and remove the other ones.
  • Remove the GPG keyring and signatures - we rely on the signatures.json files.
  • Make sure all automated PR checks pass (hard to miss, you probably won't be able to merge before the required ones are resolved).

(b) Spec linting.
@Sumynwa
Copy link
Contributor Author

Sumynwa commented Jul 25, 2022

Bumped the release version.
Spec linted.
Checked all parameters listed above.

@PawelWMS
Copy link
Contributor

PawelWMS commented Aug 3, 2022

Bumped the release version. Spec linted. Checked all parameters listed above.

Thank you for doing that! I think we still need to lint the spec a bit more because I see that the Summary tag is not the first one anymore, which is standard after running the linter. I should have mentioned more details about linting the spec. Here's more details about how to configure the one we're using (just 3 short steps). That's also what the "Spec Linting" PR check is running. The linter might change a few things in the wrong way (that's why the PR check is not required btw.) but in general it's reliable and I'll help you out with anything that the linter might get wrong. We're still tweaking it a bit.

@Sumynwa Sumynwa changed the title Moving libtommath from SPECS-EXTENDED to SPECS folder. Build dependen… Promote libtommath to SPECS Sep 15, 2022
@Sumynwa
Copy link
Contributor Author

Sumynwa commented Sep 20, 2022

@Sumynwa Sumynwa marked this pull request as draft May 22, 2023 04:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
main PR Destined for main Packaging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants