SRPM Output

[trevor-vaughan]

• 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.
• Commented out Travis tests for EOL Rubies

Closes #237

[mathieu-aubin]

try building a npm package without setting any prefix and see if it fails due to file not found
i had to add --prefix=/ to make it work if i remember correctly

[trevor-vaughan]

@mathieu-aubin I didn't change any settings except for those directly related to the RPM building.

[trevor-vaughan]

Also, I left in the support for Prefix but made it only exist if explicitly specified: https://github.com/jordansissel/fpm/pull/1657/files#diff-f32def54a1d6be460052b3c74f7aca7cR54

[mathieu-aubin]

yes, i see that. have you tried building a rpm of a npm package with your fork? When i do, it fails unless i set a prefix. On both fedora30 and 28.

will try in.a virtualenv

[trevor-vaughan]

@mathieu-aubin Can you post exactly how you're building the RPM? It's possible that we're just using two different methods and I was simply missing something during testing. I'll go ahead and update to add the Prefix in by default, simple enough and causes no issues except some warnings from rpmlint.

[mathieu-aubin]

# clone repo branch
git clone https://github.com/trevor-vaughan/fpm --branch=srpm-support && pushd fpm;

# install fpm frpm branch
make fpm-1.11.0.gem && gem install fpm-1.11.0.gem

# Failing build, without prefix
fpm \
	--output-type rpm \
	--input-type npm \
	--name webpagetest \
	--version 0.3.9 \
	--architecture noarch \
	--package webpagetest.v0.3.9-1.fc28.noarch.rpm \
	--rpm-summary='Webpagetest does web page tests' \
	--verbose --debug --debug-workspace \
	webpagetest

# Correct build, with prefix set to /
fpm \
	--prefix=/ \
	--output-type rpm \
	--input-type npm \
	--name webpagetest \
	--version 0.3.9 \
	--architecture noarch \
	--package webpagetest.v0.3.9-1.fc28.noarch.rpm \
	--rpm-summary='Webpagetest does web page tests' \
	--verbose --debug --debug-workspace \
	webpagetest

popd

i am thinking its got to do with npm prefix now... but just in case, can you double check? thanks

[mathieu-aubin]

npm config ls -l | grep prefix

gives me that /usr as prefix...

[mathieu-aubin]

I don't think that this is correct

%install
mkdir -p %{buildroot}%{prefix}
cp -r * %{buildroot}%{prefix}

that prefix (i believe) is used at install time in order to install the package in a certain directory

in fact, i just tested using a weird prefix and editing the spec file to correct thoses lines. Rpm installed into my weird prefix folder

[trevor-vaughan]

@mathieu-aubin Thanks for pinpointing this, should be corrected.

[mathieu-aubin]

this does produce what it is entended to. Now maybe, since we can keep the spec file from the srpm... how about cleaning the header a bit and some of the odd comments in order to create a cleaner srpm for distributiom.

it would be good to run a cleaning command after the " -e " or just before rpmbuild starts its work. This way they still are in the project but get cleaned only pre-packaging

[trevor-vaughan]

@mathieu-aubin If possible, I'd like to get this through as functional and then have separate work for "pretty".

This PR doesn't address all of the ideas in the full comment thread, which are very good ideas, but I needed something that worked now and I'm OK with pretty later. Many thanks for your reviews, they've been really helpful!

[mathieu-aubin]

Recommend squashin and merge
my only concern is that, altho beneficial overall, this does include some unrelated changes like typos and/or code structure changes.

[abitrolly]

@trevor-vaughan would be very nice to get Travis fix moved into separate PR to fix the repo first.

[mathieu-aubin]

This should be reviewed and merged

[sreerajkksd]

Can a maintainer take a look and merge this ?

[abitrolly]

@sreerajkksd unless somebody steps in to co-maintain, I doubt this. At the very least this PR should be rebased with no distracting changes like whitespace and commented tests.

[trevor-vaughan]

@abitrolly Rebased. Not interested in re-breaking trivial whitespace updates. Will happily add tests for detecting that a SRPM was output if you can point me at the tests that show how the systems detects that a RPM has been output.

[trevor-vaughan]

Test failures appear unrelated to this change.

[abitrolly]

Tests are not helping, because they are failing. #1681 contains fixes, and there are again no resources to review and mere that.

[jordansissel]

The tests aren’t in great shape these days, and there’s a few tests that always fail. For this PR, don’t worry about them.

…

On Sun, Mar 28, 2021 at 2:31 AM Anatoli Babenia ***@***.***> wrote: Tests are not helping, because they are failing. #1681 <#1681> contains fixes, and there are again no resources to review and mere that. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1657 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABAF2UC7VW6OM4MM46F35DTF3ZQNANCNFSM4IHIIM6Q> .

[ryysud]

Can a maintainer merge this ? cc: @jordansissel

[ryysud]

@jordansissel If you prefer to re-create PR that conflicts are resolved, I can do it. What do you think?

[trevor-vaughan]

I can rebase this again. I didn't realize there was still interest in it to be honest.

[trevor-vaughan]

My initial feedback is that I wonder if this should be a separate output type? Like --output-type srpm ?

I hadn't thought about users that wouldn't expect two RPMs to be output.

Adding a flag for the SRPM makes sense to me but it seems like you'd want to go ahead and generate it along with the main RPM to keep the logic changes small. Maybe just a --srpm flag so that you know you're going to get one output?

Folks have asked for SRPM support for years, and is this what they were asking for? It basically turns rpmbuild into a tarball -> rpm conversion tool without any meaningful "build" step. This might be ok, and is very likely what I would use it for if I had to use a system that required SRPMs. What do you think?

Well, it's the best you can get when you're not actually building from source 😄. I think it's a reasonable 80% case and actually works pretty well if you're packaging up something like a bunch of interpreted code.

The main benefit is that it gives you something that you can manually hack on later in case you want to do something more fancy than you can easily support with fpm. The other benefit is for places that simply require a SRPM for whatever reason 🤷.

Also, the resulting .src.rpm file doesn't respect the -p flag (which sets the output filename). For example:

That should be relatively easy to fix with some minor hackery. I'll work on getting it updated (hopefully in the near future).

I'll happily accept PRs onto the original if anyone else wants to help get it in there.

I wonder, could we include %_target, %_target_cpu, and %_target_os macros in the rpm spec?

Yeah, that shouldn't be too difficult.

[jordansissel]

I think it's a reasonable 80% case

Agreed

The other benefit is for places that simply require a SRPM for whatever reason 🤷.

Yes! When I think about folks requesting this feature, just having fpm output an SRPM at all would be a nice step forward for environments where a deployment step requires SRPMs as input (for whatever reason).

If I can make time, I'll send you some PRs for the changes listed in your above comment 👍

[jordansissel]

I wonder, could we include %_target, %_target_cpu, and %_target_os macros in the rpm spec?

Works! I've got this PR forked into a local branch and am working on it.

[jordansissel]

Adding a flag for the SRPM makes sense to me but it seems like you'd want to go ahead and generate it along with the main RPM to keep the logic changes small

I'm mostly focused on the behavior of the fpm command line. We can make the code simple and still have two different target packages (--output-type srpm, etc).

[jordansissel]

Small changes based on discussion are in #1952.

I'm still leaning towards having srpm be separate from rpm, and here's my thinking:, focusing on what I think the audience aka users will experience.

0. My primary expectation is that folks who want a binary RPM will continue using --output-type rpm and that this is a distinct use case that is separate from folks who want a source RPM. To me, this calls for a separate output type where a user would request an srpm specifically. Folks who want both can invoke fpm twice, once for each output type, similar to how folks do this when they want an rpm and a deb, fpm must be invoked twice. To reduce copy/paste, fpm has a number of options for "multiple output type" users to use common flags from a file.
1. At present, --output-type rpm only outputs one file with a predictable name or with a chosen path using the -p flag. We can't choose the output file path. Example below.
2. I'd be open to a flag like --rpm-also-create-srpm (or something less verbose) where --output-type rpm would still output an srpm. If we did that, it'd be nice if we could resolve the -p file path/naming issue.
3. fpm could in the future add "input" support for srpm, such as --input-type srpm, and looking at the format of those source rpms, I don't think it would prove to be much of a challenge for most srpms to convert to other formats. This is out of scope for this change, but it's in my mind.

Example of possible confusion with -p flag:

% bundle exec bin/fpm -s gem -t rpm -f -p /tmp/whatever.rpm insist
Created package {:path=>"/tmp/whatever.rpm"}

% ls /tmp/*.rpm
/tmp/rubygem-insist-1.0.0-1.src.rpm  /tmp/whatever.rpm

The -p flag only impacted the directory where the srpm was placed, but not the file name.

[jordansissel]

Thinking outloud a bit more... There are quite a number of flag options available for fpm's rpm support, and it might be annoying to have those flags doubled in the --help and documentation.

Maybe adding a new flag to let you choose rpm or srpm output? I'm still thinking that having both files output would be a bit odd and maybe confusing. Something that basically chooses rpmbuild -bb or rpmbuild -bs depending on the flag, maybe fpm --rpm-output-source ?

I personally never used rpmbuild to get a source rpm even back before fpm when I was building rpms myself. I would do rpmbuild -bb and ship the binary rpm over to mrepo and I wouldn't need the srpm ever. That said, I do know that I'm usually a weird edge case, so I'd love more input about how you (anyone!) would like to use this feature.

Would you want both a binary and source rpm from fpm at the same time?

[trevor-vaughan]

FWIW, I always generate both the SRPM and the RPM at the same time.

Since the two extensions are different, a glob of *.rpm vs *.srpm should easily take care of path expectations.

I'm certainly open to adding a flag but I think I'd prefer getting both at once to running twice in case of a possible (but admittedly unlikely) race condition between the two on a running system.

Maybe just --srpm?

[trevor-vaughan]

I should add that the reason that I generate an SRPM is that it is often required by users.

In many cases, the output of the fpm SRPM won't be "source" but, in some cases (shell, ruby, python, PERL) it might just be a valid SRPM.

[wbraswell]

Howdy @jordansissel !

Unfortunately, @NicholasBHubbard and I must strongly vote AGAINST merging this code from @trevor-vaughan (or your updated #1952), because it is providing the undesirable "fake SRPM" solution (no build instructions) instead of the desirable "real SRPM" solution (yes build instructions), as described in our original discussion during August of 2018:
#237 (comment)

This "fake SRPM" vs "real SRPM" is a key and critical distinction between an fpm-only solution (fake) and an fpm-plus-fpm-cookery solution (real). In 2018 and again yesterday, I tagged you and @bernd requesting cooperation between fpm and fpm-cookery so that we can achieve the correct solution together:
bernd/fpm-cookery#8 (comment)

Can you please reply now to let us know your intentions regarding fpm-cookery and working together toward the "real SRPM" solution?

Thanks in advance! :-)

[trevor-vaughan]

Well, I guess that nails it down to being non-default :-D.

I suppose it's probably worth hacking the short description and/or description to make sure there's a warning that this might not contain all source materials.

[NicholasBHubbard]

This PR causes FPM to output an SRPM that has an empty %build section in its spec file, meaning that you need to manually modify the spec file to add build instructions if you want to create a regular binary RPM from the SRPM.

fpm-cookery provides a "recipe" feature that allows you to specify build instructions that can be used to build the software before packaging it as a binary. Because there is no magic way to know the build commands for any particular software project, the only way we can produce a "real" source package is if a build recipe is provided as input to FPM.

I think we are better off exploring options for creating real SRPM's, hopefully by merging in the recipe feature from fpm-cookery.

[trevor-vaughan]

Hmm....I'll have to re-test, I thought that it worked correctly.

At any rate, wouldn't %build just be dumping the collected artifacts into a known location?

Should be pretty straightforward to resolve.

[NicholasBHubbard]

%build is meant to contain a command or series of commands used for building the software into machine code (for compiled languages) or byte code (for some interpreted languages).

We cannot reliably do this in a generic way for all software.

[trevor-vaughan]

Ah, yes, that's 100% true.

As an FPM user, I wouldn't expect an FPM-generated SRPM to be a "true" SRPM because it's literally impossible except in very narrow circumstances.

But I do need the ability to modify configs and things in my generated RPMs and, preferably, I can do that without having to do a song and dance with creating containers/mocks, copying files, unpacking FPM-generated RPMs, re-running fpm, etc...

[trevor-vaughan]

I guess, as long as it's opt-in and comes with warnings, I don't see the harm.

[wbraswell]

@trevor-vaughan
The fact that we will never be able to magically know what goes into %build is the very reason why the fpm-cookery project exists:
https://github.com/bernd/fpm-cookery/
This is a long-standing fpm issue that pre-dates this 2019 pull request.
The harm in accepting a fake SRPM solution is that it does not provide a real SRPM solution, as with fpm-cookery.
So again, unfortunately I must vote against merging this pull request at all. I'm really sorry, I know it is not fun to have somebody tell you that your code should not be merged. I can only point you back to the 2018 discussion which happened before this PR was made:
#237 (comment)

[trevor-vaughan]

I don't mind but using fpm-cookery seems like a lot of extra work for a minimal use case.

In my case, literally nothing goes into the %build section for my usage of FPM. It's just a bunch of files so the default behavior suffices. It would be great to be able to just shove the command line arguments at FPM and get what I need.

I guess if that means hooking fpm-cookery into fpm natively, that might work as well.

[wbraswell]

This is not a minimal use case at all, we need to build many thousands of Perl SRPMs and can not accomplish that without using fpm-cookery or equivalent.

I understand that the "fake SRPM" solution would work fine for you, but it absolutely would not work for everybody.

The original debate goes all the way back to 2012:
bernd/fpm-cookery#8

[trevor-vaughan]

Right, so I'm asking that I get the option for my use case, and you don't have to use it for yours, that's pretty much it.

[NicholasBHubbard]

I don't think that there is anything wrong with creating an SRPM with an empty %build section if no build recipe is supplied, but @wbraswell and I believe that it is necessary to have the option to supply a build recipe to create an SRPM with a proper %build section.

[trevor-vaughan]

Being able to supply a %build section certainly seems reasonable.

I definitely think that this discussion nails things down to 'do not generate an SRPM by default' :-D

[jordansissel]

I hear you. I don’t think this change prevents any particular future changes, so I’m not sure it’s useful to treat this as an “either-or” scenario. This solution might not be what you want, but that doesn’t mean your problem can’t be solved in the future :) As for “real srpm”, let’s have that discussion in a new issue? I’d like to gather more details on your use cases, workflows, and tools, and I’d like us to move away from the terms of “real” or “fake” as these are subjective and aren’t helping the discussion.

…

On Fri, Nov 4, 2022, at 10:01 AM, William N. Braswell, Jr. wrote: @trevor-vaughan <https://github.com/trevor-vaughan> The fact that we will never be able to magically know what goes into `%build` is the very reason why the fpm-cookery project exists: https://github.com/bernd/fpm-cookery/ This is a long-standing fpm issue that pre-dates this 2019 pull request. The harm in accepting a fake SRPM solution is that it does not provide a real SRPM solution, as with fpm-cookery. So again, unfortunately I must vote against merging this pull request at all. I'm really sorry, I know it is not fun to have somebody tell you that your code should not be merged. I can only point you back to the 2018 discussion which happened before this PR was made: #237 (comment) <#237 (comment)> — Reply to this email directly, view it on GitHub <#1657 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABAF2WMVJX7FKQJAD22Y5LWGU6P7ANCNFSM4IHIIM6Q>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[wbraswell]

@jordansissel
Okay I will open a new issue.

An SRPM with an empty %build can not be built, and thus defeats the primary purpose of an SRPM existence.
This is the basis of my usage of the term "fake SRPM", because it does not provide the actual functionality of a real SRPM.
Per your request, I will instead use the terms "buildable SRPM" and "unbuildable SRPM" from now on.

[jordansissel]

Added flag --rpm-with-source defaulting to false so that the old behavior remains the default and that no SRPM is produced by default. I also added a feature where fpm will try to match the output file name (whatever.rpm) with the source rpm filename.

# An rpm file named "test" should get an srpm named "test.src.rpm"
% bundle exec bin/fpm -s empty -t rpm -n example -p /tmp/test --rpm-with-source
Created SRPM {:path=>"/tmp/test.src.rpm"}
Created package {:path=>"/tmp/test"}

# An rpm named "test.rpm" should get an srpm named "test.src.rpm"
% bundle exec bin/fpm -s empty -t rpm -n example -p /tmp/test.rpm --rpm-with-source
Created SRPM {:path=>"/tmp/test.src.rpm"}
Created package {:path=>"/tmp/test.rpm"}

# Default name and using --rpm-with-source
% bundle exec bin/fpm -s empty -t rpm -n example --rpm-with-source              
Created SRPM {:path=>"./example-1.0-1.src.rpm"}
Created package {:path=>"example-1.0-1.noarch.rpm"}

# Defaults (no SRPM)
bundle exec bin/fpm -s empty -t rpm -n example 
Created package {:path=>"example-1.0-1.noarch.rpm"}

[jordansissel]

I think we should be OK to merge this (#1952, which is this PR plus a few small changes).

@wbraswell I don't think this change (here, or #1952) will negatively impact work on #1954. If anything, maybe it sets the stage that source packages are possible from fpm.