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

Add simplicity options to pkg build system #6274

Merged
merged 19 commits into from
Oct 26, 2021

Conversation

ocket8888
Copy link
Contributor

@ocket8888 ocket8888 commented Oct 13, 2021

Adds the following options to pkg:

  • -S suppress outputting "source RPMs"
  • -s use simple RPM names e.g. traffic_ops.rpm instead of traffic_ops-6.1.0-11637.ec9ff6a6.el8.x86_64.rpm
  • -L suppress outputting logs in files (no e.g. traffic_ops_build.log)
  • and the -h option to print help text

Previously, help text was output whenever an unknown option was passed. Now, passing unknown options causes pkg to exit with a failure and print the offending option(s) followed by usage information, while -h causes it to exit with a success after printing usage information.

The hope is to eventually use this to make the Makefile for CDN-in-a-Box much, much simpler, and to enable it to build the RPMs and emplace them properly without access to the repo's git information in situations where it may not be accessible (as is the case in e.g. extracted release tarballs).


Which Traffic Control components are affected by this PR?

  • documentation

None/all, depending on your point of view. Only build system files were changed.

What is the best way to verify this PR?

Make sure that pkg still works normally when these aren't passed. Specifically, I checked the following:

  • calling pkg with no arguments and/or options builds all RPMs and SRPMs, with build logs and the normal, long-form RPM filenames
  • calling pkg with no options while specifying a single project builds only the RPMs and SRPMs for that project, with build logs and the normal, long-form RPM filenames
  • calling pkg with -h prints usage information and exits with code 0
  • calling pkg with an unrecognized option prints the option(s) it didn't recognize, followed by usage information, and then exits with a non-zero code
  • calling pkg with a single project specified and -S option given outputs only an RPM and build log for that project, no SRPM, with the normal, long-form RPM filename
  • calling pkg with a single project specified and -s option given outputs only the RPM and SRPM for that project, with build logs, but using simple filenames containing no version information, only the project name.
  • calling pkg with a single project specified and -L option given outputs only the RPM and SRPM for that project, without build logs, and using the normal, long-form RPM filenames
  • calling pkg with a single project specified and -S and -s given outputs only the RPM and build log for that project, no SRPM, and using a simple RPM filename containing no version information, only the project name.
  • calling pkg with a single project specified and -S and -L given outputs only an RPM for the given project, no SPRM or build log, and uses the normal, long-form RPM filename
  • calling pkg with a single project specified and -s and -L given outputs only the RPM and SRPM for the given project, without a build log, and using simple RPM filenames containing no version information, only the project name.
  • calling pkg with a single project specified and -S, -s, and -L given outputs only an RPM for the given project, without an SRPM or build log, and using a simple RPM file name containing no version information, only the project name.

PR submission checklist

  • Existing tests are sufficient, as the new behavior is not depended upon as part of the regular, documented, supported build process.
  • This PR has documentation
  • This PR has a CHANGELOG.md entry
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY (see the Apache Software Foundation's security guidelines for details)

@ocket8888 ocket8888 added build related to the build process documentation related to documentation improvement The functionality exists but it could be improved in some way. labels Oct 13, 2021
@zrhoffman
Copy link
Member

Does this fix #6273?

@ocket8888
Copy link
Contributor Author

Does this fix #6273?

No, it leaves that behavior unchanged

@ocket8888 ocket8888 force-pushed the pkg-simple-packages branch from b8153a6 to 1bce131 Compare October 22, 2021 16:22
Copy link
Collaborator

@tcfdev tcfdev left a comment

Choose a reason for hiding this comment

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

Verified all functionality works and previous functionality remains.

build/functions.sh Outdated Show resolved Hide resolved
@zrhoffman
Copy link
Member

I am still reviewing this

Copy link
Member

@zrhoffman zrhoffman left a comment

Choose a reason for hiding this comment

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

  • grove, grovetccfg, and traffic_router still include the build number in the RPM filename when -s is specified.

  • It was undocumented, but previously, --help would print the help text, too. Now, it prints

    ./pkg: illegal option -- -
    ./pkg: illegal option -- e
    

    followed by the help text printed to stderr, with exit code 1.

build/functions.sh Show resolved Hide resolved
pkg Outdated Show resolved Hide resolved
pkg Outdated Show resolved Hide resolved
build/clean_build.sh Show resolved Hide resolved
pkg Outdated Show resolved Hide resolved
pkg Outdated Show resolved Hide resolved
pkg Outdated Show resolved Hide resolved
pkg Outdated Show resolved Hide resolved
pkg Outdated Show resolved Hide resolved
build/functions.sh Outdated Show resolved Hide resolved
@ocket8888
Copy link
Contributor Author

ocket8888 commented Oct 22, 2021

It was undocumented, but previously, --help would print the help text, too. Now, it prints ...

That's because it was still parsing those as illegal, unrecognized options. You could also see the help text by passing --spaghetti. getopt does not support GNU-style long-options.

@zrhoffman
Copy link
Member

getopt does not support GNU-style long-options.

Fair enough, if we ever add support for other long options, we can add --help then, too.

@ocket8888 ocket8888 force-pushed the pkg-simple-packages branch 2 times, most recently from 770cf03 to 533443b Compare October 25, 2021 17:39
Copy link
Member

@zrhoffman zrhoffman left a comment

Choose a reason for hiding this comment

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

All of the additional changes look good. Besides the flag sort order, the only outstanding review comment is

  • grove, grovetccfg, and traffic_router still include the build number in the RPM filename when -s is specified.

pkg Outdated Show resolved Hide resolved
@ocket8888
Copy link
Contributor Author

  • grove, grovetccfg, and traffic_router still include the build number in the RPM filename when -s is specified.

grove, grovetccfg, and tomcat also were building but not outputting source RPMs. I fixed it so their respective source RPMs are copied to dist (when not disabled).

Adds the -S option to suppress outputting "source RPMs", the -s option
to use simple RPM names, the -L option to suppress outputting log s in
files, and the -h option to print help text.

Previously, help text was output whenever an unknown option was passed.
Now, passing unknown options causes pkg to exit with a failure and print
the offending option followed by usage information, while -h causes it
to exit with a success after printing usage information.
@ocket8888 ocket8888 force-pushed the pkg-simple-packages branch from bea8af2 to b90149b Compare October 26, 2021 14:28
Copy link
Member

@zrhoffman zrhoffman left a comment

Choose a reason for hiding this comment

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

It looks like -s only applies to RPMs right now. Should it apply to tarballs, too?

apache-trafficcontrol-6.1.0-docs.tar.gz
apache-trafficcontrol-6.1.0.tar.gz

@ocket8888
Copy link
Contributor Author

It could, for sure. Would be consistent. I just doubt there's a lot of value in that, since the tarball has to be constructed from the source tree anyway, and idk how that could be useful outside of making a release.

@zrhoffman
Copy link
Member

Okay, maybe someone can make that improvement in a future PR.

Copy link
Member

@zrhoffman zrhoffman left a comment

Choose a reason for hiding this comment

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

  • -S omits source RPMs
  • -s removes version-release-architecture from the RPM filename
  • -L suppresses logs
  • -h shows help text
  • Existing functionality, besides --spaghetti, remains intact

@zrhoffman zrhoffman merged commit e2ba960 into apache:master Oct 26, 2021
@ocket8888 ocket8888 deleted the pkg-simple-packages branch October 27, 2021 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build related to the build process documentation related to documentation improvement The functionality exists but it could be improved in some way.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants