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

SRPM output (continuation of #1657) #1952

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

SRPM output (continuation of #1657) #1952

wants to merge 13 commits into from

Conversation

jordansissel
Copy link
Owner

Continuation of #1657 from @trevor-vaughan's work.

trevor-vaughan and others added 9 commits November 3, 2022 23:22
* Modify RPM build process to generate and collect SRPM and RPM
  artifacts by default
* Create a spec file that will allow for a natural
  `rpmbuild --rebuild <thing>.src.rpm` approach
* Ensure that required components are present in the RPM spec file
* At this time, not feature flagged. If users don't want the src.rpm
  output, they simply don't have to use it.

Closes #237
* Remove deprecated Buildroot option from the RPM template
When building an rpm with rpmbuild, the default cpu architecture seems
to be the host cpu. However, fpm's default architecture is what rpm
calls `noarch`, so to achieve this, I found that we need to set the
_target_cpu value to the same one used by fpm and the same that
we would use for the `--target` rpmbuild flag.
This avoids copying files from one temporary path (staging_path) to
another path rpmbuild build path.

This doesn't necessarily solve any particular bug. I thought this
copying appeared a bit unnecessary and tried using the staging_path as
the _builddir. It seems to work :)

The code history was that the rpm spec %install section was copying the
files. Then, in PR #552, fpm copies files into rpm's build dir and the
%install was made a no-op. As of this commit, no extra copying happens.
This reduces the lines of code used to generate the source tarball.
@jordansissel
Copy link
Owner Author

@trevor-vaughan Hello again! I'm open to however you'd like to handle this. We can discuss here or if you prefer back over on the fpm PR you filed.

Changes from your PR:

  • Sets %_target_cpu to match what fpm's --architecture flag is set to. This allows rpmbuild --rebuild to be used without a --target flag which is kinda nice.
  • Minor rewrite of the source tarball creation to be less lines of code.
  • Unrelated to your PR: Removes an old change that copies files from fpm's staging_path to the expected default rpmbuild _builddir. Instead, fpm now sets the _builddir rpmbuild setting directly to fpm's staging path. This skips some file copies which hopefully is nice?

The fpm test suite for rpm passes ✅

% bundle exec rspec spec/fpm/package/rpm_spec.rb
...................................................................

Finished in 8.8 seconds (files took 0.20296 seconds to load)
67 examples, 0 failures

Testing expectations, I would expect fpm and rpmbuild --rebuild to produce roughly the same rpm -- with exception that the build time and other external variables might be different.

Testing this:

% bundle exec bin/fpm -s gem -t rpm insist
Created package {:path=>"rubygem-insist-1.0.0-1.noarch.rpm"}

% rpmbuild --rebuild rubygem-insist-1.0.0-1.src.rpm
Installing rubygem-insist-1.0.0-1.src.rpm
...
Wrote: /home/jls/rpmbuild/RPMS/noarch/rubygem-insist-1.0.0-1.noarch.rpm
...

Cool. So far so good. Both filenames are the same and rpmbuild used the same target cpu (noarch). Let's check metadata:

% rpm -qip  /home/jls/rpmbuild/RPMS/noarch/rubygem-insist-1.0.0-1.noarch.rpm | grep -v 'Build Date' | md5sum
2569eb460def6afe12fec24da2047923  -

% rpm -qip rubygem-insist-1.0.0-1.noarch.rpm | grep -v 'Build Date' | md5sum
2569eb460def6afe12fec24da2047923  -

Checksums for both rpms match (if we exclude 'Build Date'). Nice!

Checking file lists:

% rpm -qlp rubygem-insist-1.0.0-1.noarch.rpm |md5sum
4c51f7d593cfab693b0c9b424a48ff6c  -
% rpm -qlp /home/jls/rpmbuild/RPMS/noarch/rubygem-insist-1.0.0-1.noarch.rpm |md5sum
4c51f7d593cfab693b0c9b424a48ff6c  -

File names listing are identical

There's timestamp differences on the files, but that might also be due to external clock aka build time variables.

% rpm -qvlp /home/jls/rpmbuild/RPMS/noarch/rubygem-insist-1.0.0-1.noarch.rpm | head -1
drwxr-xr-x    2 root     root                        0 Nov  3 23:29 /home/jls/.rbenv/versions/3.1.0/lib/ruby/gems/3.1.0/build_info

% rpm -qvlp rubygem-insist-1.0.0-1.noarch.rpm | head -1
drwxr-xr-x    2 root     root                        0 Nov  3 23:28 /home/jls/.rbenv/versions/3.1.0/lib/ruby/gems/3.1.0/build_info

@jordansissel jordansissel mentioned this pull request Nov 4, 2022
@wbraswell
Copy link
Contributor

@jordansissel
Sorry, I do not believe this pull request should be merged!
Please see the link below for more information & discussion:
#1657 (comment)

This is necessary for cases where someone sets the architecture to
something rpmbuild doesn't know about (like the fpm test suite uses
"fancypants").

Fixes #1956 and only seemed to affect the work done in #237
This is for cases like `fpm -p hello.rpm` and it seems like we'd want
the srpm named "hello.src.rpm" instead of whatever was generated by
rpmbuild. I don't know if this will make things less confusing, but I
hope it will make things more predictable :)
This flag allows a user to ask for an SRPM in addition to the binary
rpm.

This also adds fallback case if the file extension seems weird. That is,
if someone asks for a package file named "foobar" then fpm will also
output an SRPM named "foobar.src.rpm" in addition to the rpm file named
"foobar"
Otherwise, built just the binary rpm
@jordansissel jordansissel marked this pull request as ready for review November 14, 2022 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants