There are concerns that replacing all make invocations with %make_build, even in cases where there may not be any benefit e.g. for make html may cause build failures or make the spec file less readable.

Those concerns were mine.

An alternative to replacing all make invocations would be only to replace make in make $BUILD_TARGET, make check, and make install with %make_{build,install}.

@tstellar Do you want FESCo to decide here?

An alternative to replacing all make invocations would be only to replace make in make $BUILD_TARGET, make check, and make install with %make_{build,install}.

@tstellar Do you want FESCo to decide here?

Yes, it would be helpful to get input from FESCo on this.

I'm basically only in favor of making make %{?_smp_mflags} -> %make_build and make DESTDIR=%{buildroot} install -> %make_install substitutions. The rest, I feel very iffy about just going and changing.

+1 to @ngompa

@ngompa I agree. For some things (e.g. Miro's example of building sphinx docs) don't make sense to parallelize. Your examples should be the two cases where it's completely fine to basically just sed s/old/new.

There's probably also cases of "pure" make invocations without arguments, and in those cases, it should be fine to change to %make_build as well, unless there's an associated comment in the .spec file that the parallel build has issues (but this is probably not scriptable).

I do not think this really gains a lot and makes things less clear to contributors by reading the spec file. I agree that there are instances where these macros don't make sense. Making the changes described here encodes a lot of deep understanding and makes it less obvious to new contributors when they start out and read spec files for the first time.

From a historical standpoint.... many moons ago spec files used to use %makeinstall which ran 'make install' and overrode common variables like PREFIX and BINDIR and such. Then DESTDIR became more common so we changed everything to 'make DESTIDR=%{buildroot} install' then we changed to '%{__make} DESTDIR=%{buildroot} install' because you could override the __make command locally or on a builder and pass -j. But that made sense globally, so it became a global macro we called %{?_smp_mflags} so that then changed to 'make %{?_smp_mflags} DESTDIR=%{buildroot} install'. But then a lot of Makefiles are poorly written and don't handle parallel jobs for the install target, so let's drop that and go back to 'make DESTDIR=%{buildroot} install'. And now we want to shorten that to '%make_install'?

Are we really gaining a lot here? I'm all for standards and reproducible builds, but does this one make things too opaque for new contributors?

[Also, I am aware of the %{buildroot} vs. $RPM_BUILD_ROOT style difference. I always preferred the former in spec files to make it clear I was referring to a variable provided by rpmbuild. In my examples above, $RPM_BUILD_ROOT was used interchangeably with %{buildroot} depending on the package.]

(...) 'make DESTDIR=%{buildroot} install'. And now we want to shorten that to '%make_install'?

@dcantrell, note that this is, by all means, not "new", the "general" Packaging Guidelines - and the CMake Packaging Guidelines in particular - have referenced %make_build and %make_install for ages already.

Are we really gaining a lot here? I'm all for standards and reproducible builds, but does this one make things too opaque for new contributors?

For most cases, this Change only brings .spec files more closely into alignment with current Packaging Guidelines, so as far as I am concerned, that's better for new contributors (because documentation and actual usage get aligned).

After 7 days, there are no votes. Since there seem to be a need for a specific decision, I'm tagging with meeting, so we can brainstrom ideas to unblock this there.

We've talked about this ticket in this meeting:

https://meetbot.fedoraproject.org/fedora-meeting-2/2020-07-08/fesco.2020-07-08-14.01.html

The following was APPROVED (+6, 2, -0):

Neal's proposal in https://pagure.io/fesco/issue/2420#comment-662515 and ask change owner to reopen discussion if they think more should happen than that.

With that, the change proposal is effectively declined. Sorry about that.

@churchyard What are the next steps for me? Should I update the proposal to match
https://pagure.io/fesco/issue/2420#comment-662515 and then begin implementing it? Or do I need to wait until F34 now?

@tstellar I think if you update it to match, then it is fine to do in F33. Also my slight preference if we make it self-contained because there is not going to be any breakage or anything, it is quite small, known scope.

You can update the proposal or follow https://docs.fedoraproject.org/en-US/fesco/Mass_package_changes/ if you still like to work on this. My understanding was that you would not be interested, sorry if that was a mistake on my part.

You can update the proposal or follow https://docs.fedoraproject.org/en-US/fesco/Mass_package_changes/ if you still like to work on this. My understanding was that you would not be interested, sorry if that was a mistake on my part.

On the mailing list thread, I did say that I felt the modified proposal was not all the beneficial. However, I've changed my mind after reviewing the spec files and finding there were a lot more make %{?_smp_mflags} in spec files than I thought. So, I am interested in implementing the modified version of the proposal.

OK, please edit the proposal and let us know when it is ready.

@churchyard I have finished updating the proposal.

+1

+1

+1

https://fedoraproject.org/w/index.php?title=Changes%2FUseMakeBuildInstallMacro&type=revision&diff=581904&oldid=581754

+1

My earlier assent refers to the diff that @churchyard is presenting now.

+1

After a week, I count the vote as (+5, 0, -0). Processing the proposal as accepted.

Edited to remove a negative vote that didn't exist

Announced.