-
Notifications
You must be signed in to change notification settings - Fork 346
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
Build development RPMs from any OS without needing Docker #4758
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8b206e3
to
7f6dc8b
Compare
Rebased to fix merge conflicts. |
I tested out these changes locally and they work great for me. 👍 |
srijeet0406
approved these changes
Jun 23, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested the changes on my local machine and they worked fine 👍
Now that #4671 is merged, rebased to support |
- Almquist shellcheck - Error trap - Strict shell options - Return 1 from functions on error instead of exit 1 since we have an error trap and the errexit option set
- Single brackets for Bourne shell, not double - Use -n instead of ! -z - Use braces for variables inside strings
treat 0 as a successful exit code instead of 1
support an arbitrary number of files
editing RPM definitions
… already run - Do not copy dist directory - Gitignore grove and grovetccfg binaries
- Specify that we want to build for Linux, regardless of host OS - Use awk to print the first field of wc, since BSD wc starts with a tab - Support stat -c on BSD systems - npm, bower, and grunt are already in the path, do not prefix with /usr/bin/ - Specify recursive copy with -R, not -r - Use ${file.separator} in Traffic Router POMs - If the build environment is Cygwin, use a Windows GOPATH
stop the build script - Specify environment requirements by component
The Traffic Portal Developer Environment" section
- Move Compass instructions to "Software Requirements" section - Add reference labels
- Change directories to the repo before running if we are not already there - Clean build the tarball to exclude gitignored files - If no projects are provided to clean_build.sh, build them all
- Fix label references - Fix some typos - Link to the Windows footnote from the Chocolatey package row
- Use the sphinx module to build the docs, not the sphinx-build executable - check for python3, not python
Rebased onto master |
7 tasks
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR (Pull Request) do?
sh
) instead of Bash for POSIX compatibilityrealpath
instead ofreadlink
where possible02:54.01
00:53.44
00:44.94
00:43.69
00:50.65
01:11.01
01:49.90
01:44.56
00:55.07
11:47.27
02:11.94
00:15.17
00:13.53
00:06.10
00:15.39
00:24.18
01:01.34
00:49.34
00:16.31
05:33.30
01:53.61
00:03.38
00:03.97
00:04.36
00:04.93
00:10.40
00:45.23
00:33.98
00:07.98
03:47.84
01:14.69
00:02.68
00:03.18
00:02.91
00:03.37
00:07.65
00:27.99
00:18.32
00:06.12
02:26.9
Some comments about the time comparisons table:
./pkg -v [component]
, Natively refers tobuild/clean_build [component]
/tmp/go/src/github.com/apache/trafficcontrol
, so thersync
command inclean_build.sh
to copy the repo there only affects files that have changed since the last native build../pkg
command bootstrapsdocker-compose
, which takes ~1.6 seconds./tmp/go/src/github.com/apache/trafficcontrol
is not a volume, thersync
command inclean_build.sh
always copies the entire repo.rsync
takes on average ~3.2 seconds.Binary size: Static linking vs dynamic linking
Since this PR makes the Go binaries statically linked, whereas before they were dynamically linked, I expected the binaries to be a little larger, but it ends up saving a very small amount of space instead.
7261
9358
11189
18473
10077
9983
7223
9300
11127
18404
10019
9917
38
58
62
69
58
66
Some comments about the static vs. dynamic linking table:
go build -ldflags '-s -w'
and were then stripped of symbols (strip [binary name]
)go build -ldflags '-s -w' -tags 'osusergo netgo'
and were then stripped of symbols (strip [binary name]
)Besides being a tiny bit smaller, the static binaries will load trivially faster at runtime. However, our statically-linked Go binaries on average take about 1.2 seconds longer to build each.
Which Traffic Control components are affected by this PR?
What is the best way to verify this PR?
Verify that the target OS of the RPMs is Linux
Expected output:
Verify that the RPM compression format is XZ
Expected output:
Verify that all of the packaged Go binaries are statically linked (have no dynamic section). Example for
traffic_ops_golang
:docker run --rm -v$(pwd):/mnt centos:7 readelf -d /mnt/traffic_ops_golang
Expected output is
In the CDN-in-a-Box directory, run
make
to build the RPMs and verify that CiaB worksdocs/source/development/building.rst
under Build the RPMs Natively, follow the Install the Dependencies instructions for your OSmake native
to build the RPMs and verify that CiaB works./pkg -v
(Docker), verify the RPMs and gzipped TARs that CDN-in-a-Box does not use:build/clean_build.sh
(native), verify the RPMs and gzipped TARs that CDN-in-a-Box does not useIf this is a bug fix, what versions of Traffic Control are affected?
For the environment check bugfix:
For #3527:
Otherwise, not a bug fix.
The following criteria are ALL met by this PR