versat
Refactor of RPM packaging files + migrate from sysvinit -> systemd service
This PR introduces the following changes:
- Use new specfile derived from the Fedora EPEL upstream
- Use native systemd units for service management
- Build with debugging symbols at all times as they are stripped out in install or by RPM
- Explicitly set CC to gcc during RPM build
- Increment version number to 0.95.0beta1
- Build + test RPM packages in RHEL + SuSE containers as opposed to under Ubuntu in GH actions
Hello @gs-kamnas, can you better explain this PR?
@fralken sure; my overall goals in this change are to use a native systemd service for managing cNTLM when run as a Linux system service, given that most major Linux distros are using systemd as the initsystem as well as simplify the RPM specfile and associated build process via the usage of macros present in recent versions of RPM.
Furthermore, I opted to amend the CI based workflow to better test the end result by building and installing the RPM package in both a RHEL as well as SuSE based container environment.
That's interesting! Have you checked the option to run a job within a container? Maybe it's possible to avoid maintaining the dockerfiles by implementing specific jobs (that is, by splitting the "build" job in the c-cpp.yml file in separate jobs depending on different linux distros)
That's interesting! Have you checked the option to run a job within a container? Maybe it's possible to avoid maintaining the dockerfiles by implementing specific jobs (that is, by splitting the "build" job in the c-cpp.yml file in separate jobs depending on different linux distros)
This looks like it will work however will of course bind the RPM build task more tightly to github actions as opposed to being able to run it standalone. If this is preferable I can certainly refactor to the above.
This looks like it will work however will of course bind the RPM build task more tightly to github actions as opposed to being able to run it standalone. If this is preferable I can certainly refactor to the above.
This is a valid point, but when is the need to build the rpm locally not running on a RHEL or Suse host?
This looks like it will work however will of course bind the RPM build task more tightly to github actions as opposed to being able to run it standalone. If this is preferable I can certainly refactor to the above.
This is a valid point, but when is the need to build the rpm locally not running on a RHEL or Suse host?
Very niche case agreed, therefore refactored to use Github actions as requested as that will likely be a bit easier to maintain long term.
hello @gs-kamnas and @jschwartzenberg . Is it ready to merge this PR?
@gs-kamnas are you able to verify also the debian package?
hello @gs-kamnas and @jschwartzenberg . Is it ready to merge this PR?
@gs-kamnas are you able to verify also the debian package?
Would prefer to close out the systemd user vs. system scoped but parameterized with username discussion 1st; then I can absolutely verify that the Debian package functions as well.
Would you prefer I do so against any particular version of Debian, or is the latest OK?
Would prefer to close out the systemd user vs. system scoped but parameterized with username discussion 1st; then I can absolutely verify that the Debian package functions as well.
Would you prefer I do so against any particular version of Debian, or is the latest OK?
Of course, first complete this PR. As for the deb package, I guess it is OK to work on the latest. It is important that it can be installed in the major Debian derivatives (mainly Ubuntu).
I never investigated too much on rpm and deb packages because I'm using cntlm mainly on windows and mac.
Can you also rebase this PR on the latest master branch?
Rebased as well as refactored as per @jschwartzenberg 's feedback,
Therefore will get on testing this comprehensively under the latest versions of RHEL, SLES as well as Debian. Afterwards, I believe we are good to consider merging.
Hello, I see that the rpmbuild is quite inconsistent among distributions, I tested on Centos, Opensuse and Fedora and I get different results. When i run make rpm:
-
on Centos, I get three artifacts: cntlm-0.95.0beta1-1.el9.x86_64.rpm cntlm-debuginfo-0.95.0beta1-1.el9.x86_64.rpm cntlm-debugsource-0.95.0beta1-1.el9.x86_64.rpm
The cntlm binary size is ~638KB
-
on Opensuse, I get one artifact: cntlm-0.95.0beta1-1.x86_64.rpm
The cntlm binary size is ~3.4MB (debug info are not stripped)
-
on Fedora, the build fails. This is because
rpmbuildreplaces the CFLAGS with completely different one and tries linking against libraries that are not available.
Is there a way to make rpmbuild to produce the same artifact on various distributions?
Hello, I see that the
rpmbuildis quite inconsistent among distributions, I tested on Centos, Opensuse and Fedora and I get different results. When i runmake rpm:
- on Centos, I get three artifacts: cntlm-0.95.0beta1-1.el9.x86_64.rpm cntlm-debuginfo-0.95.0beta1-1.el9.x86_64.rpm cntlm-debugsource-0.95.0beta1-1.el9.x86_64.rpm The cntlm binary size is ~638KB
- on Opensuse, I get one artifact: cntlm-0.95.0beta1-1.x86_64.rpm The cntlm binary size is ~3.4MB (debug info are not stripped)
- on Fedora, the build fails. This is because
rpmbuildreplaces the CFLAGS with completely different one and tries linking against libraries that are not available.Is there a way to make
rpmbuildto produce the same artifact on various distributions?
Definitely should be doable, therefore will look into making this more consistent (and as close as possible to the RHEL behavior)
@fralken It appears to be rather straightforward to disable the CFLAGS, LDFLAGS and related environment variable overrides by setting %undefine _auto_set_build_flags in the specfile per https://src.fedoraproject.org/rpms/redhat-rpm-config/blob/rawhide/f/buildflags.md#using-rpm-build-flags
Furthermore, debug symbols extraction is an opt-in feature in the SLES RPM macro implementation (that I now conditionally enable if on SLES).
Therefore, I now believe the behavior with regards to the build flags used and binaries generated to be far more consistent across distros (which has the added benefit of also remediating the build failure on Fedora).
Finally, I have added a verification step in the GH actions pipeline to assert that the installed binaries are stripped, as well as log the size of such.
@fralken It appears to be rather straightforward to disable the CFLAGS, LDFLAGS and related environment variable overrides by setting
%undefine _auto_set_build_flagsin the specfile per https://src.fedoraproject.org/rpms/redhat-rpm-config/blob/rawhide/f/buildflags.md#using-rpm-build-flagsFurthermore, debug symbols extraction is an opt-in feature in the SLES RPM macro implementation (that I now conditionally enable if on SLES).
Therefore, I now believe the behavior with regards to the build flags used and binaries generated to be far more consistent across distros (which has the added benefit of also remediating the build failure on Fedora).
Finally, I have added a verification step in the GH actions pipeline to assert that the installed binaries are stripped, as well as log the size of such.
This is great! thanks a lot! I was testing with fedora but I don't know if there are any other major linux distributions that use RPM to test upon.
Also, If we release the packages, which rpm build should be released?
In my opinion what is still missing is clean up the repo a bit.
- on "make clean" remove intermediate artifacts and folders created during build
- it is a better practice not to automatically change commit files. For example "rpm/SPECS/cntlm.spec" is changed with the version name. It is better to commit a "rpm/SPECS/cntlm.spec.in" template and create cntlm.spec on the fly
- same thing for "debian/changelog"
- don't mix files required for packaging with source code. That is "cntlm-user.in" should be put in a subfolder. Maybe it is convenient to have a linux subfolder and move deb and rpm subfolders there and also "cntlm-user.in"
Finally, it would be nice, for better documenting, to squash intermediate commits and leave the relevant ones. And also rebase on master.
@fralken Agree with regards to recommended changes, therefore will implement over next several days,
Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
No data about Coverage
No data about Duplication
See analysis details on SonarCloud
![sonarqubecloud[bot] avatar](https://avatars.githubusercontent.com/in/12526?v=4 "Published by a pub.dev verified publisher"){kind=small}
Clean / distclean target in makefile should now be working as desired. Also, I believe I have handled all cases where the build was introducing modifications to the source tree.
Rebase is still pending as I may also want to review some of the written documentation for accuracy as well.
Hello @gs-kamnas are you planning to complete this PR? It is almost there, just a small refactoring is needed.
Apologies for the radio silence, too many competing priorities therefore I haven't had the time to dedicate to this for a while. Will aim to address the documentation items that are outstanding as well as rebase this coming week.
@fralken I consider this ready for a re-review when you have the time. Thanks in advance!
Hello @gs-kamnas, thanks for the update. I see that there are still 3 (out of 4) pending reviews in the Makefile file.
Also, it would be nice if you could squash the commits and leave the relevant ones, so it is easier to review the various changes. Thanks!
Acknowledged with regards to squashing down the number of commits to a more reasonable/relevant number.
However, I cannot see your comments on the Makefile @fralken ; are you certain you have submitted your review?
Screenshot:
Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
See analysis details on SonarCloud
Acknowledged with regards to squashing down the number of commits to a more reasonable/relevant number.
However, I cannot see your comments on the Makefile @fralken ; are you certain you have submitted your review?
I'm sorry, my fault, I forgot to press "submit review".