Clean up builds and stop publishing non-LFS RPMs #54
Conversation
This file contains hidden or 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
In commit git-lfs/git-lfs@e642e53 of PR git-lfs/git-lfs#5169 we dropped support for Debian 9 (stretch) in our primary Git LFS project, so we drop it here now as well, for the reasons explained that commit's description: Debian 9 (stretch) is now beyond LTS support and on to ELTS support, which means that the typical Debian infrastructure is no longer involved and updates are no longer available from Debian. As a result, it's not really appropriate for us to continue to build packages on stretch since we can't be certain our dependencies or build environment are secure. It is the case that some of the OSes that use Debian 9 packages, such as some versions of Ubuntu, do still receive security updates, but since we don't build specifically for those OSes, we'll need to drop support for them as well. Of the OSes receiving mainline (non-extended) security support, only Ubuntu bionic is affected.
Our Dockerfiles are not executable binaries or scripts, so we remove the executable file permissions on these files.
In commit 833d085 of PR git-lfs#47 we updated our Debian Dockerfiles so they would install the asciidoctor package, to allow our primary Git LFS project to migrate from ronn to asciidoctor. Now that that migration is complete, as of PR git-lfs/git-lfs#5054, we can update our Debian Dockerfiles again so they no longer install any ronn packages.
Since commit 7af5552 in PR git-lfs#19 in 2018 our CentOS 7 Dockerfile has installed Git 2.16.0. As that version is now somewhat old, we update to a recent version instead, i.e., 2.34.5. We avoid using a newer version higher then 2.34.x because those include a "test balloon" check for C99 compiler support, as introduced in git/git@7bc341e, which fails without further changes to our CentOS 7 build.
Since the original introduction of support for building CentOS RPMs in commit git-lfs/git-lfs@4a71627 of PR git-lfs/git-lfs#332, an rpm/clean.bsh script has been provided to perform the equivalent of a "git clean -xdf" command on the Git repository from which we build Git LFS. However, this script is no longer needed as all of the CentOS and Rocky Linux container image builds have a native Git package installed, so we remove our use of it, and also drop the script from the primary Git LFS project in PR git-lfs/git-lfs#5241.
We use an rsync command to copy our built RPM packages into the output directory specified by the REPO_DIR variable, which defaults to /repo. At present, this command copies all the RPM files, which includes the ones we built for the asciidoctor Ruby gem (and prior to Git LFS v3.3.0, the ones we built for the hpricot, mustache, rdiscount, and ronn Ruby gems). As a result we publish RPMs such as rubygem-asciidoctor-2.0.17-1.el9.src.rpm and rubygem-asciidoctor-2.0.17-1.el9.noarch.rpm in our Packagecloud account. To exclude these RPMs so we do not upload them to our Packagecloud account, we add --include and --exclude options to the rsync command which limits copying to files with a "git-lfs" prefix.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Dec 28, 2022
Since the original introduction of support for building CentOS RPMs in commit 4a71627 of PR git-lfs#332, an rpm/clean.bsh script has been provided to perform the equivalent of a "git clean -xdf" command on the Git repository from which we build Git LFS. However, this script is no longer needed as all of the CentOS and Rocky Linux container image builds have a native Git package installed, and so we remove the only use of it, in the centos_script.bsh script of the git-lfs/build-dockers project, in commit git-lfs/build-dockers@7be51a4 of PR git-lfs/build-dockers#54, and can therefore also drop the script from this repository as well.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Dec 28, 2022
In commit c2d25ee of PR git-lfs#511 we added support for building RPM packages for 32-bit platforms by updating the docker/centos_script.bsh script which was present at that time to call rpmbuild with a --target=i686 argument. Since commit 56ffe42 of PR git-lfs#555 both that script and the rpm/build_rpms.bsh script contained the same logic to parse the OS name and version in order to set a short suffix for the RPM filenames. However, the docker/centos_script.bsh script was subsequently moved into the git-lfs/build-dockers repository, where it has not been updated to match the rpm/build_rpms.bsh script, such as when parsing of the OS major version was added in commit e939409 of PR git-lfs#5054, which allows us to properly parse the version number on CentOS/Rocky Linux 8 and above, or when parsing of the Rocky Linux OS name was added in commit 723be34 of PR git-lfs#5144. The result is that at present we build 32-bit RPMs for CentOS 8 and Rocky Linux 9 (el8 and el9, respectively) without the platform short name suffix in their filenames, e.g., git-lfs-3.3.0-1.i686.rpm and git-lfs-3.3.0-1.i686.rpm, and then upload them to Packagecloud with those names. To resolve this problem and avoid later regressions between the two sets of parsing logic, we move the rpmbuild command for 32-bit packages into our rpm/build_rpms.bsh script, which ensures they will be built with the same context as our 64-bit packages. To do this we introduce an rpmbuild command with the --target=i686 argument into rpm/build_rpms.bsh, which allows us to also remove the rpmbuild command from the centos_script.bsh script in the git-lfs/build-dockers repository in PR git-lfs/build-dockers#54.
In commit git-lfs/git-lfs@c2d25ee of PR git-lfs/git-lfs#511 we added support for building RPM packages for 32-bit platforms by updating the docker/centos_script.bsh script which was present at that time to call rpmbuild with a --target=i686 argument. Since commit git-lfs/git-lfs@56ffe42 of PR git-lfs/git-lfs#555 both that script and the rpm/build_rpms.bsh script in the primary Git LFS project's repository contained the same logic to parse the OS name and version in order to set a short suffix for the RPM filenames. However, the docker/centos_script.bsh script was subsequently moved into this repository, where it has not been updated to match the rpm/build_rpms.bsh script in the primary project repository, such as when parsing of the OS major version was added in commit git-lfs/git-lfs@e939409 of PR git-lfs/git-lfs#5054, which allows us to properly parse the version number on CentOS/Rocky Linux 8 and above, or when parsing of the Rocky Linux OS name was added in commit git-lfs/git-lfs@723be34 of PR git-lfs/git-lfs#5144. The result is that at present we build 32-bit RPMs for CentOS 8 and Rocky Linux 9 (el8 and el9, respectively) without the platform short name suffix in their filenames, e.g., git-lfs-3.3.0-1.i686.rpm and git-lfs-3.3.0-1.i686.rpm, and then upload them to Packagecloud with those names. To resolve this problem and avoid later regressions between the two sets of parsing logic, we add an rpmbuild command with the --target=i686 argument for 32-bit packages to the rpm/build_rpms.bsh script in the primary Git LFS project repository in commit git-lfs/git-lfs@7830f04 of PR git-lfs/git-lfs#5241, which ensures they will be built with the same context as our 64-bit packages. We can therefore also remove the rpmbuild command with the --target=i686 argument from the centos_script.bsh script in this repository, along with the setup code which attempted to parse the OS version and name from either /etc/os-release or /etc/redhat-release, as these values are only used to set the RPM_DIST variable passed to rpmbuild, and the initialization code for that variable can be removed as well.
bk2204
approved these changes
Jan 3, 2023
Member
Author
|
As a followup, I've deleted all the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This PR cleans up various issues with our Docker container image definitions and build scripts, and in particular removes our leftover support for Debian 9, stops our accidental publication to Packagecloud of RPMs unrelated to Git LFS, and defers to the primary Git LFS project's RPM build script to handle 32-bit builds so as to fix a naming problem with the 32-bit RPMs for CentOS 8 and Rocky Linux 9.
Reviewing this PR commit-by-commit is recommended, and each commit should have a full description of its changes.
We expect to merge this PR only when its companion PR git-lfs/git-lfs#5241 has been reviewed and merged, specifically because that PR adds support for 32-bit RPM builds.
This PR's changes have been tested in conjunction with those of PR git-lfs/git-lfs#5241 and confirmed to produce valid build artifacts. We used a branch of
git-lfs/git-lfswith the changes from PR git-lfs/git-lfs#5241 plus revisions to the GitHub Actions workflows so as to use this PR's branch and then to upload the resultant artifacts to GitHub Actions. These can be examined to confirm that they now correctly name the 32-bit RPM builds for CentOS 8 and Rocky Linux 9 and that they also include only Git LFS packages and no unrelated ones:Note that after these PRs are merged, we should delete the unrelated RPMs from our Packagecloud account to save some space and cost.
Removing Debian 9 Support
In commit git-lfs/git-lfs@e642e53 of PR git-lfs/git-lfs#5169 we dropped support for Debian 9 (stretch) in our primary Git LFS project, so we drop it here now as well, for the reasons explained that commit's description:
One advantageous consequence of this change is that we should see the
Build Linux packagesCI job in the primary Git LFS project run somewhat faster, as it currently calls thebuild_dockers.bshscript without specifying an architecture with an--archargument, and so that script then builds containers for all Docker instruction files matching*.Dockerfile, which means we are currently building an extra, unused Debian 9 container for thedebian_9.Dockerfilefile on every CI run.Unintentionally Published RPM Packages
We use an
rsynccommand to copy our built RPM packages into the output directory specified by theREPO_DIRvariable, which defaults to/repo.At present, this command copies all the RPM files, which includes the ones we built for the
asciidoctorRuby gem (and prior to Git LFS v3.3.0, the ones we built for thehpricot,mustache,rdiscount, andronnRuby gems). As a result we publish RPMs such asrubygem-asciidoctor-2.0.17-1.el9.src.rpmandrubygem-asciidoctor-2.0.17-1.el9.noarch.rpmin our Packagecloud account.To exclude these RPMs so we do not upload them to our Packagecloud account, we add
--includeand--excludeoptions to thersynccommand which limits copying to files with agit-lfsprefix.Let Primary Project Script to Build 32-Bit RPMs
In commit git-lfs/git-lfs@c2d25ee of PR git-lfs/git-lfs#511 we added support for building RPM packages for 32-bit platforms by updating the
docker/centos_script.bshscript which was present at that time in the primary Git LFS project's repository to callrpmbuildwith a--target=i686argument.Since commit git-lfs/git-lfs@56ffe42 of PR git-lfs/git-lfs#555 both that script and the
rpm/build_rpms.bshscript in the primary project's repository contained the same logic to parse the OS name and version in order to set a short suffix for the RPM filenames.However, the
docker/centos_script.bshscript was subsequently moved into this repository, where it has not been updated to match therpm/build_rpms.bshscript in the primary project repository, such as when parsing of the OS major version was added in commit git-lfs/git-lfs@e939409 of PR git-lfs/git-lfs#5054, which allows us to properly parse the version number on CentOS/Rocky Linux 8 and above, or when parsing of the Rocky Linux OS name was added in commit git-lfs/git-lfs@723be34 of PR git-lfs/git-lfs#5144.The result is that at present we build 32-bit RPMs for CentOS 8 and Rocky Linux 9 (
el8andel9, respectively) without the platform short name suffix in their filenames, e.g.,git-lfs-3.3.0-1.i686.rpmandgit-lfs-3.3.0-1.i686.rpm(instead ofgit-lfs-3.3.0-1.el9.i686.rpmandgit-lfs-3.3.0-1.el8.i686.rpm), and then upload them to Packagecloud with those names.To resolve this problem and avoid later regressions between the two sets of parsing logic, we add an
rpmbuildcommand with the--target=i686argument for 32-bit packages to therpm/build_rpms.bshscript in the primary Git LFS project repository in commit git-lfs/git-lfs@7830f04 of PR git-lfs/git-lfs#5241, which ensures they will be built with the same context as our 64-bit packages.We can therefore also remove the
rpmbuildcommand with the--target=i686argument from thecentos_script.bshscript in this repository, along with the setup code which attempted to parse the OS version and name from either/etc/os-releaseor/etc/redhat-release, as these values are only used to set theRPM_DISTvariable passed torpmbuild, and the initialization code for that variable can be removed as well.Removing Substitute Repository Cleanup Script
Since the original introduction of support for building CentOS RPMs in commit git-lfs/git-lfs@4a71627 of PR git-lfs/git-lfs#332, an
rpm/clean.bshscript has been provided to perform the equivalent of agit clean -xdfcommand on the Git repository from which we build Git LFS.However, this script is no longer needed as all of the CentOS and Rocky Linux container image builds have a native Git package installed (or a recent Git built, in the case of CentOS 7), so we remove our use of it, and also drop the script from the primary Git LFS project in commit git-lfs/git-lfs@ba6f278 of PR git-lfs/git-lfs#5241.
Use Git 2.34.x in CentOS 7 RPM Build
Since commit 7af5552 in PR #19 in 2018 our CentOS 7
Dockerfilehas installed Git 2.16.0. As that version is now somewhat old, we update to a recent version instead, i.e., 2.34.5.We avoid using a newer version higher then 2.34.x because those include a "test balloon" check for C99 compiler support, as introduced in git/git@7bc341e, which fails without further changes to our CentOS 7 build.