#1 Fixes for various bugs found during mass rebuild
Closed 2 years ago by zbyszek. Opened 2 years ago by zbyszek.
rpms/ zbyszek/package-notes fixes-for-mass-rebuild  into  rawhide

file modified
+2 -2
@@ -3,6 +3,6 @@ 

  # for details.

  %_package_note_file     %{_builddir}/.package_note-%{name}-%{version}-%{release}.%{_arch}.ld

  

- %_package_note_flags    %{?name:-Wl,-dT,%{_package_note_file}}

+ %_package_note_flags    %{?_package_note_file:%{?name:%["%_target_cpu" == "noarch"?"":"-Wl,-dT,%{_package_note_file}"]}}

  

- %_generate_package_note_file %{?name: if ! [ -f %{_package_note_file} ] && [ -f %{_rpmconfigdir}/generate-rpm-note.sh ]; then %{_rpmconfigdir}/generate-rpm-note.sh %{name} %{version}-%{release} %{_arch} >%{_package_note_file}; fi}

+ %_generate_package_note_file %{?_package_note_file:%{?name:%["%_target_cpu" == "noarch"?"":"if [ -f %{_rpmconfigdir}/generate-rpm-note.sh ]; then %{_rpmconfigdir}/generate-rpm-note.sh ${RPM_PACKAGE_NAME:?} ${RPM_PACKAGE_VERSION:?}-${RPM_PACKAGE_RELEASE:?} ${RPM_ARCH:?} >%{_package_note_file}; fi"]}}

no initial comment

This should be fixed as well, right?

BTW should it really be hidden file? And does it need to be in %{_builddir}

And also, shouldn't the whole feature be disable for noarch packages? Two reasons:

1) It is not useful for noarch packages, because they don't use ELF, where this would be embedded
2) It refers to builder architecture, which for noarch packages does not make sense or would be even confusing

This should be fixed as well, right?

I assume that by "fixed" you mean "conditionalized on name". The problem is that if I just wrap the RHS in %{name?: … }, I end up with an empty macro. This doesn't seem to help with anything, and could be confusing. If there's a syntax to define the whole macro conditionally, I'd use that.

BTW should it really be hidden file? And does it need to be in %{_builddir}

I'll paste my reply from https://bugzilla.redhat.com/show_bug.cgi?id=2043092:

It's written to %{_builddir}, so we usually end up with something reasonable like /builddir/build/BUILD/.package_note-lirc-0.10.0-34.fc36.x86_64.ld.
I would love to push it one level down, e.g. to /builddir/build/BUILD/lirc-0.10.0/.package_note-lirc-0.10.0-34.fc36.x86_64.ld, or even /builddir/build/BUILD/lirc-0.10.0/.package_note.x86_64.ld. Unfortunately rpm does not have a concept of a separate build directory under %{_builddir}, and it's up to each and every package to make a directory there. This usually happens, but not always, and even if it does, there is no macro to reference the name, so we have no choice but to use %{_builddir}.

--

And also, shouldn't the whole feature be disable for noarch packages?

It seems it should. I'll rework it like that.

This should be fixed as well, right?

I assume that by "fixed" you mean "conditionalized on name".

No, there need to be used the env variables to have a proper version/release

This should be fixed as well, right?

I assume that by "fixed" you mean "conditionalized on name".

No, there need to be used the env variables to have a proper version/release

IOW the filename is still wrong

1 new commit added

  • Also voidify the macros if we're on a noarch build
2 years ago

No, there need to be used the env variables to have a proper version/release

Oh, OK. But I don't think this can be done, because the place where the macro will be used might not do variable expansion. For example, if you want to to show the notes in the build, and you do something like cat '%_package_note_file', this should work. Both the %_package_note_file and %_package_note_flags macros are "plain text", after the macro expansion has been performed. People want to inject the build flags into various places, and expect them to be an already expanded text. Trying to do variable expansion again would be wrong, because the expansion might not be idempotent.

Yes, this means that the file name will be "wrong" in some cases, like the ruby builds. But this should be only a small subset of packages, and it's misleading but should have no practical effect. For normal distro builds %_builddir is initialized from scratch, so the uniqueness of the name doesn't matter.

No, there need to be used the env variables to have a proper version/release

Oh, OK. But I don't think this can be done, because the place where the macro will be used might not do variable expansion.

But in that case, wouldn't it be better to drop the VR? The issue on the beginning was confusing enough, like what is happening and the confusing VR did not help.

BTW wouldn't be something like %(echo ${RPM_PACKAGE_VERSION:?}) feasible?

BTW wouldn't be something like %(echo ${RPM_PACKAGE_VERSION:?}) feasible?

I'm testing this now…

BTW wouldn't be something like %(echo ${RPM_PACKAGE_VERSION:?}) feasible?

I'm testing this now…

Nope, it seems that RPM_PACKAGE_VERSION is set too late. I get "sh: line 1: RPM_PACKAGE_VERSION: parameter null or not set" with this.

Pull-Request has been closed by zbyszek

2 years ago
Metadata