Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@Conan-Kudo commented on this pull request. > +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF THIRD PARTY > RIGHTS. IN +# NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +# DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +# OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE +# OR OTHER DEALINGS IN THE SOFTWARE. +# +# Except as contained in this notice, the name of a copyright holder shall not +# be used in advertising or otherwise to promote the sale, use or other dealings +# in this Software without prior written authorization of the copyright holder. +# + +set -e -o pipefail + +getos() { +# shellcheck disable=SC1091 +test -r /etc/os-release && . /etc/os-release That makes sense to me. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#discussion_r1032990015 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@keszybz commented on this pull request. > +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF THIRD PARTY > RIGHTS. IN +# NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +# DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +# OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE +# OR OTHER DEALINGS IN THE SOFTWARE. +# +# Except as contained in this notice, the name of a copyright holder shall not +# be used in advertising or otherwise to promote the sale, use or other dealings +# in this Software without prior written authorization of the copyright holder. +# + +set -e -o pipefail + +getos() { +# shellcheck disable=SC1091 +test -r /etc/os-release && . /etc/os-release OK. So something like this: ```bash if test -e /etc/os-release; then . /etc/os-release elif test -e /usr/lib/os-release; then . /usr/lib/os-release fi ``` Differences from the original code: - do not suppress permission errors by using `-r` instead of `-e` - use /usr/lib/os-release if appropriate -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#discussion_r1032988938 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@keszybz commented on this pull request. > + +printf 'Installed-Build-Depends:\n' >> "$BUILDINFO" +rpm -qa --queryformat '%{epoch}:%{name}-%{version}-%{release}.%{arch}\n' \ +| LC_ALL=C sort -t: -k2 \ +| sed -e 's/^(none)://; /\.(none)$/d; s/^/ /' >> "$BUILDINFO" + +printf 'Environment:\n' >> "$BUILDINFO" + +# Whitelist from Debian's Dpkg: +# https://anonscm.debian.org/git/dpkg/dpkg.git/tree/scripts/Dpkg/Build/Info.pm#n50 +ENV_WHITELIST= + +# Toolchain. +ENV_WHITELIST="$ENV_WHITELIST CC CPP CXX OBJC OBJCXX PC FC M2C AS LD AR RANLIB MAKE AWK LEX YACC" +# Toolchain flags. +ENV_WHITELIST="$ENV_WHITELIST CFLAGS CPPFLAGS CXXFLAGS OBJCFLAGS OBJCXXFLAGS GCJFLAGS FFLAGS LDFLAGS ARFLAGS MAKEFLAGS" The environment in various stages of the build depends on various packages in the build environment, so I don't think we should try to capture it. It shoud be reproduced when the same packages are installed. But it seems that `rpm` passes the environment passed in from the outside into the build environment, so capturing that seems useful. `/proc/self/environ` sounds like a reasonable option. OTOH, if the builds are done in `mock`, the environment would be sanitized anyway. `mock` now uses `systemd-nspawn`, and it standarizes the environment to a large degree. Nevertheless, it does inject some information, like `container_host_*`. This was added in https://github.com/systemd/systemd/pull/15891/commits/e1bb4b0d1dd91ea13792e9d16d4bbf4ab8385f58. I wonder if we should add a switch to systemd-nspawn to opt out of setting those variables and make mock use it. The environment inside of mock should not care at all about the host. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#discussion_r1032988061 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@Conan-Kudo commented on this pull request. > +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF THIRD PARTY > RIGHTS. IN +# NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +# DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +# OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE +# OR OTHER DEALINGS IN THE SOFTWARE. +# +# Except as contained in this notice, the name of a copyright holder shall not +# be used in advertising or otherwise to promote the sale, use or other dealings +# in this Software without prior written authorization of the copyright holder. +# + +set -e -o pipefail + +getos() { +# shellcheck disable=SC1091 +test -r /etc/os-release && . /etc/os-release Your conditional assumes the file exists at all. It needs to handle when it's not present at all. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#discussion_r1032985079 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@Conan-Kudo commented on this pull request. > + +printf '%s' "\ +Format: 1.0 +BuildArchitecture: ${RPM_BUILD_ARCH} +Name: ${RPM_PACKAGE_NAME} +Epoch: ${RPM_PACKAGE_EPOCH} +Version: ${RPM_PACKAGE_VERSION} +Release: ${RPM_PACKAGE_RELEASE} +Architecture: ${RPM_ARCH} +BuildOS: ${RPM_BUILD_OS} +BuildDir: ${RPM_BUILD_DIR} +" > "${BUILDINFO}" + +{ \ +printf 'EnvironmentRequires:\n'; \ +rpm -qa --queryformat '%{epoch}:%{name}-%{version}-%{release}.%{arch}\n' \ That old ordering is probably influenced by the old Debian tooling using YUM instead of DNF, and YUM was weird about this. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#discussion_r1032984842 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@keszybz commented on this pull request. > +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF THIRD PARTY > RIGHTS. IN +# NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +# DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +# OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE +# OR OTHER DEALINGS IN THE SOFTWARE. +# +# Except as contained in this notice, the name of a copyright holder shall not +# be used in advertising or otherwise to promote the sale, use or other dealings +# in this Software without prior written authorization of the copyright holder. +# + +set -e -o pipefail + +getos() { +# shellcheck disable=SC1091 +test -r /etc/os-release && . /etc/os-release os-release is also /usr/lib/os-release. Please use this instead: ```sh test -e /etc/os-release && os_release='/etc/os-release' || os_release='/usr/lib/os-release' . "${os_release}" ``` > + +printf '%s' "\ +Format: 1.0 +BuildArchitecture: ${RPM_BUILD_ARCH} +Name: ${RPM_PACKAGE_NAME} +Epoch: ${RPM_PACKAGE_EPOCH} +Version: ${RPM_PACKAGE_VERSION} +Release: ${RPM_PACKAGE_RELEASE} +Architecture: ${RPM_ARCH} +BuildOS: ${RPM_BUILD_OS} +BuildDir: ${RPM_BUILD_DIR} +" > "${BUILDINFO}" + +{ \ +printf 'EnvironmentRequires:\n'; \ +rpm -qa --queryformat '%{epoch}:%{name}-%{version}-%{release}.%{arch}\n' \ Hmm, why this ordering. I would expect: '%{name}-%{epoch}:%{version}-%{release}.%{arch}\n' (It seems `--queryformat` doesn't support more complex expressions, so the sed for `(none):` still seems needed ;( ) -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#pullrequestreview-1195032934 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@DemiMarie commented on this pull request. > +# in this Software without prior written authorization of the copyright > holder. +# + +set -e -o pipefail + +getos() { +# shellcheck disable=SC1091 +test -r /etc/os-release && . /etc/os-release +if test -z "${ID}"; then +ID="$(cat /etc/system-release)" +fi +printf '%s' "${ID}" +} + +RPM_BUILD_ROOT="$1" +RPM_BUILD_DIR="$2" A separate buildinfo RPM seems like the best option to me. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#discussion_r1032852444 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@Conan-Kudo commented on this pull request. > +# in this Software without prior written authorization of the copyright > holder. +# + +set -e -o pipefail + +getos() { +# shellcheck disable=SC1091 +test -r /etc/os-release && . /etc/os-release +if test -z "${ID}"; then +ID="$(cat /etc/system-release)" +fi +printf '%s' "${ID}" +} + +RPM_BUILD_ROOT="$1" +RPM_BUILD_DIR="$2" Oh that's true... -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#discussion_r1032852130 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@marmarek commented on this pull request. > +# in this Software without prior written authorization of the copyright > holder. +# + +set -e -o pipefail + +getos() { +# shellcheck disable=SC1091 +test -r /etc/os-release && . /etc/os-release +if test -z "${ID}"; then +ID="$(cat /etc/system-release)" +fi +printf '%s' "${ID}" +} + +RPM_BUILD_ROOT="$1" +RPM_BUILD_DIR="$2" > If we want to stick it somewhere, the debugsource package might be as good of > a place as any? Do all packages have debugsource? AFAIK noarch usually do not, right? -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#discussion_r1032846295 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@Conan-Kudo commented on this pull request. > +# in this Software without prior written authorization of the copyright > holder. +# + +set -e -o pipefail + +getos() { +# shellcheck disable=SC1091 +test -r /etc/os-release && . /etc/os-release +if test -z "${ID}"; then +ID="$(cat /etc/system-release)" +fi +printf '%s' "${ID}" +} + +RPM_BUILD_ROOT="$1" +RPM_BUILD_DIR="$2" If we want to stick it somewhere, the debugsource package might be as good of a place as any? -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#discussion_r1032838156 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
Closed #1532. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#event-7866567762 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
This doesn't seem to go anywhere. So I am closing this PR here. Feel free to re-open or may be open a new one. The discussion is becoming a bit unwieldy. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#issuecomment-1323478239 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
Thank you all for your feedback, I'll prepare another iteration soon with all the comments here. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#issuecomment-943021678___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@fepitre commented on this pull request. > + +printf 'Installed-Build-Depends:\n' >> "$BUILDINFO" +rpm -qa --queryformat '%{epoch}:%{name}-%{version}-%{release}.%{arch}\n' \ +| LC_ALL=C sort -t: -k2 \ +| sed -e 's/^(none)://; /\.(none)$/d; s/^/ /' >> "$BUILDINFO" + +printf 'Environment:\n' >> "$BUILDINFO" + +# Whitelist from Debian's Dpkg: +# https://anonscm.debian.org/git/dpkg/dpkg.git/tree/scripts/Dpkg/Build/Info.pm#n50 +ENV_WHITELIST= + +# Toolchain. +ENV_WHITELIST="$ENV_WHITELIST CC CPP CXX OBJC OBJCXX PC FC M2C AS LD AR RANLIB MAKE AWK LEX YACC" +# Toolchain flags. +ENV_WHITELIST="$ENV_WHITELIST CFLAGS CPPFLAGS CXXFLAGS OBJCFLAGS OBJCXXFLAGS GCJFLAGS FFLAGS LDFLAGS ARFLAGS MAKEFLAGS" Ah I see. Thank you for the enlightenment. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#discussion_r728673531___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@Conan-Kudo commented on this pull request. > + +mkdir -p "$BUILDINFO_DIR" + +cat > "$BUILDINFO" <> "$BUILDINFO" I'm fine with another filename. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#discussion_r728462928___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@Conan-Kudo commented on this pull request. > + +printf 'Installed-Build-Depends:\n' >> "$BUILDINFO" +rpm -qa --queryformat '%{epoch}:%{name}-%{version}-%{release}.%{arch}\n' \ +| LC_ALL=C sort -t: -k2 \ +| sed -e 's/^(none)://; /\.(none)$/d; s/^/ /' >> "$BUILDINFO" + +printf 'Environment:\n' >> "$BUILDINFO" + +# Whitelist from Debian's Dpkg: +# https://anonscm.debian.org/git/dpkg/dpkg.git/tree/scripts/Dpkg/Build/Info.pm#n50 +ENV_WHITELIST= + +# Toolchain. +ENV_WHITELIST="$ENV_WHITELIST CC CPP CXX OBJC OBJCXX PC FC M2C AS LD AR RANLIB MAKE AWK LEX YACC" +# Toolchain flags. +ENV_WHITELIST="$ENV_WHITELIST CFLAGS CPPFLAGS CXXFLAGS OBJCFLAGS OBJCXXFLAGS GCJFLAGS FFLAGS LDFLAGS ARFLAGS MAKEFLAGS" What @voxik is saying is that you will not be able to capture the variables because they only exist in the `%build` step. We'd need a hook to export it from there. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#discussion_r728462636___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
Koji has a similar build environment record, though it's stored in the Koji database rather than as a file. We do archive environment artifacts from Mock with builds too, though. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#issuecomment-942735441___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@woju commented on this pull request. > + +mkdir -p "$BUILDINFO_DIR" + +cat > "$BUILDINFO" <> "$BUILDINFO" If you really insist on a different format for RPM, I'd suggest the file suffix be changed to something else than `.buildinfo` (maybe `.rpmbuildinfo`?). This will at least make distinguishing the files easier. Archlinux has `.BUILDINFO` (all caps) I think. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#discussion_r728443050___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@woju commented on this pull request. > +Format: 1.0-rpm +Build-Architecture: $(uname -m) +Source: $RPM_PACKAGE_NAME +Epoch: $RPM_PACKAGE_EPOCH +Version: ${RPM_PACKAGE_VERSION} +Release: ${RPM_PACKAGE_RELEASE} +Architecture: $RPM_ARCH +Build-Origin: $(getos) +Build-Path: $RPM_BUILD_DIR > I don't think that is valuable for us, given that we have properties in RPM > that don't exist in Debian Can you please suggest which properties those are exactly, and specifically how differences in their content might affect reproducibility of the packages? The buildinfo file was purposefully designed to *not* include all available information, only the relevant to reproducible builds, because recording too much would not actually be useful when analysing (un)reproducibility. The underlying assumption is that the build process of a package will be made as robust as possible, that is, allow as much variability in environment as reasonably possible, which in theory should allow to record less information in buildinfo. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#discussion_r728440187___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
Interested in something like this for openSUSE. We already have something comparable called `_buildenv` (XML) e.g. in https://build.opensuse.org/package/binaries/openSUSE:Factory/bash/standard - but that is created on the obs_worker level. [ArchLinux](https://archlinux.org/pacman/BUILDINFO.5.html) seems to have their own format with = as delimiter. In most places, the buildinfo files can probably be handled as opaque files and just things like source,binary,version would be interesting outside. That is not hard to extract in either format, so there is no strong reason to follow Debian here. OTOH, this is not a .spec file, but just another output from the build process, similar to the build log, so it does not need to follow rpm conventions either. So from my view, using either the Debian or ArchLinux (marshalling) format has some advantage over making up a third format. Though, I would not keep confusing Debianisms in keys that will not be meaningful outside of the respective distribution. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#issuecomment-942694178___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@fepitre commented on this pull request. > + +mkdir -p "$BUILDINFO_DIR" + +cat > "$BUILDINFO" <> "$BUILDINFO" @Conan-Kudo thank you for your feedback and comments. Generally I would have loved to have a generic format not being one distro specific to ease manipulating this file among several rebuild tools but I guess it would not be straightforward. I'm waiting some feedback from @bmwiedemann too then I would propose to adapt the work according to your comments. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#discussion_r728399795___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@bmwiedemann do you have some feedback to give from several comments here? As you work on reproducible builds for openSUSE you are certainly interested by this new feature. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#issuecomment-942664681___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@fepitre commented on this pull request. > + +printf 'Installed-Build-Depends:\n' >> "$BUILDINFO" +rpm -qa --queryformat '%{epoch}:%{name}-%{version}-%{release}.%{arch}\n' \ +| LC_ALL=C sort -t: -k2 \ +| sed -e 's/^(none)://; /\.(none)$/d; s/^/ /' >> "$BUILDINFO" + +printf 'Environment:\n' >> "$BUILDINFO" + +# Whitelist from Debian's Dpkg: +# https://anonscm.debian.org/git/dpkg/dpkg.git/tree/scripts/Dpkg/Build/Info.pm#n50 +ENV_WHITELIST= + +# Toolchain. +ENV_WHITELIST="$ENV_WHITELIST CC CPP CXX OBJC OBJCXX PC FC M2C AS LD AR RANLIB MAKE AWK LEX YACC" +# Toolchain flags. +ENV_WHITELIST="$ENV_WHITELIST CFLAGS CPPFLAGS CXXFLAGS OBJCFLAGS OBJCXXFLAGS GCJFLAGS FFLAGS LDFLAGS ARFLAGS MAKEFLAGS" I don't understand your point. This is not to set with %configure. It's the job of rebuilder like https://github.com/fepitre/rpmreproduce -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#discussion_r728396243___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@voxik commented on this pull request. > + +printf 'Installed-Build-Depends:\n' >> "$BUILDINFO" +rpm -qa --queryformat '%{epoch}:%{name}-%{version}-%{release}.%{arch}\n' \ +| LC_ALL=C sort -t: -k2 \ +| sed -e 's/^(none)://; /\.(none)$/d; s/^/ /' >> "$BUILDINFO" + +printf 'Environment:\n' >> "$BUILDINFO" + +# Whitelist from Debian's Dpkg: +# https://anonscm.debian.org/git/dpkg/dpkg.git/tree/scripts/Dpkg/Build/Info.pm#n50 +ENV_WHITELIST= + +# Toolchain. +ENV_WHITELIST="$ENV_WHITELIST CC CPP CXX OBJC OBJCXX PC FC M2C AS LD AR RANLIB MAKE AWK LEX YACC" +# Toolchain flags. +ENV_WHITELIST="$ENV_WHITELIST CFLAGS CPPFLAGS CXXFLAGS OBJCFLAGS OBJCXXFLAGS GCJFLAGS FFLAGS LDFLAGS ARFLAGS MAKEFLAGS" These will not provide any relevant information, unless you set them, which is typically done via call to `%configure` macro in `%build` section. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#pullrequestreview-778183386___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@Conan-Kudo commented on this pull request. > +Format: 1.0-rpm +Build-Architecture: $(uname -m) +Source: $RPM_PACKAGE_NAME +Epoch: $RPM_PACKAGE_EPOCH +Version: ${RPM_PACKAGE_VERSION} +Release: ${RPM_PACKAGE_RELEASE} +Architecture: $RPM_ARCH +Build-Origin: $(getos) +Build-Path: $RPM_BUILD_DIR Also of note, the name `Source:` instead of `Name:` is not clear, so that clearly would also have to change. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#discussion_r727437546___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@Conan-Kudo commented on this pull request. > + +mkdir -p "$BUILDINFO_DIR" + +cat > "$BUILDINFO" <> "$BUILDINFO" And I'd rather not have Debian-isms in here. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#discussion_r727428772___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@woju commented on this pull request. > + +mkdir -p "$BUILDINFO_DIR" + +cat > "$BUILDINFO" <> "$BUILDINFO" Again, that's debianism ([https://manpages.debian.org/bullseye/dpkg-dev/deb-buildinfo.5.en.html#Installed-Build-Depends:](https://manpages.debian.org/bullseye/dpkg-dev/deb-buildinfo.5.en.html#Installed-Build-Depends:)) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#discussion_r727421835___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@woju commented on this pull request. > +Format: 1.0-rpm +Build-Architecture: $(uname -m) +Source: $RPM_PACKAGE_NAME +Epoch: $RPM_PACKAGE_EPOCH +Version: ${RPM_PACKAGE_VERSION} +Release: ${RPM_PACKAGE_RELEASE} +Architecture: $RPM_ARCH +Build-Origin: $(getos) +Build-Path: $RPM_BUILD_DIR > You're defining an rpm-based format. This is actually a debian-defined format[^1] and at the time we was mocking this format[^2] we thought it would be good to maintain compatibility, because we expect much of the tooling will be shared. Obviously things like package version specification will be RPM-specific (hence `-rpm` in `Format:`), but the rest, like exchange, signature verification, comparison etc. does not need to be different. So the original intention is to maintain as close format to the original as possible. [^1]: https://wiki.debian.org/ReproducibleBuilds/BuildinfoFiles, https://manpages.debian.org/bullseye/dpkg-dev/deb-buildinfo.5.en.html [^2]: https://github.com/woju/rpmbuildinfo during [Reproducible Builds Summit 2016](https://reproducible-builds.org/events/berlin2016/) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#discussion_r727421752___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@Conan-Kudo requested changes on this pull request. > +Format: 1.0-rpm +Build-Architecture: $(uname -m) +Source: $RPM_PACKAGE_NAME +Epoch: $RPM_PACKAGE_EPOCH +Version: ${RPM_PACKAGE_VERSION} +Release: ${RPM_PACKAGE_RELEASE} +Architecture: $RPM_ARCH +Build-Origin: $(getos) +Build-Path: $RPM_BUILD_DIR `1.0-rpm` makes no sense in this context. You're defining an rpm-based format. RPM convention is to use CamelCase rather than hyphenation, so please be consistent with that (e.g. `BuildArchitecture` vs `Build-Architecture`). Please use RPM names (`BuildOS` vs `Build-Origin`, `BuildDir` vs `Build-Path`, etc.). Also, please consistently call shell variables with `${}` format. > + +mkdir -p "$BUILDINFO_DIR" + +cat > "$BUILDINFO" <> "$BUILDINFO" This is not an appropriate name for this, since you are describing the environment. It's _not_ just various `BuildRequires`. I would suggest `EnvironmentRequires`. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#pullrequestreview-777270029___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
@DemiMarie requested changes on this pull request. Avoid using “whitelist” and properly quote variables containing special characters > +cat > "$BUILDINFO" < "$BUILDINFO" ``` This ensures that we properly abort on errors. > +rpm -qa --queryformat '%{epoch}:%{name}-%{version}-%{release}.%{arch}\n' \ +| LC_ALL=C sort -t: -k2 \ +| sed -e 's/^(none)://; /\.(none)$/d; s/^/ /' >> "$BUILDINFO" Sadly this requires `-o pipefail` if we want to handle errors robustly. > +Epoch: $RPM_PACKAGE_EPOCH +Version: ${RPM_PACKAGE_VERSION} +Release: ${RPM_PACKAGE_RELEASE} +Architecture: $RPM_ARCH +Build-Origin: $(getos) +Build-Path: $RPM_BUILD_DIR +EOF + +printf 'Installed-Build-Depends:\n' >> "$BUILDINFO" +rpm -qa --queryformat '%{epoch}:%{name}-%{version}-%{release}.%{arch}\n' \ +| LC_ALL=C sort -t: -k2 \ +| sed -e 's/^(none)://; /\.(none)$/d; s/^/ /' >> "$BUILDINFO" + +printf 'Environment:\n' >> "$BUILDINFO" + +# Whitelist from Debian's Dpkg: ```suggestion # Allowlist from Debian's Dpkg: ``` The terms “whitelist” and “blacklist” are offensive and should not be used. > +# > https://anonscm.debian.org/git/dpkg/dpkg.git/tree/scripts/Dpkg/Build/Info.pm#n50 +ENV_WHITELIST= + +# Toolchain. +ENV_WHITELIST="$ENV_WHITELIST CC CPP CXX OBJC OBJCXX PC FC M2C AS LD AR RANLIB MAKE AWK LEX YACC" +# Toolchain flags. +ENV_WHITELIST="$ENV_WHITELIST CFLAGS CPPFLAGS CXXFLAGS OBJCFLAGS OBJCXXFLAGS GCJFLAGS FFLAGS LDFLAGS ARFLAGS MAKEFLAGS" +# Dynamic linker, see ld(1). +ENV_WHITELIST="$ENV_WHITELIST LD_LIBRARY_PATH" +# Locale, see locale(1). +ENV_WHITELIST="$ENV_WHITELIST LANG LC_ALL LC_CTYPE LC_NUMERIC LC_TIME LC_COLLATE LC_MONETARY LC_MESSAGES LC_PAPER LC_NAME LC_ADDRESS LC_TELEPHONE LC_MEASUREMENT LC_IDENTIFICATION" +ENV_WHITELIST="$ENV_WHITELIST SOURCE_DATE_EPOCH" +for var in $ENV_WHITELIST +do +eval value="\$$var" +# shellcheck disable=SC2154 +test -n "$value" && printf ' %s="%s"\n' "$var" "$value" >> "$BUILDINFO" +done ```suggestion # https://anonscm.debian.org/git/dpkg/dpkg.git/tree/scripts/Dpkg/Build/Info.pm#n50 ENV_ALLOWLIST= # Toolchain. ENV_ALLOWLIST="$ENV_ALLOWLIST CC CPP CXX OBJC OBJCXX PC FC M2C AS LD AR RANLIB MAKE AWK LEX YACC" # Toolchain flags. ENV_ALLOWLIST="$ENV_ALLOWLIST CFLAGS CPPFLAGS CXXFLAGS OBJCFLAGS OBJCXXFLAGS GCJFLAGS FFLAGS LDFLAGS ARFLAGS MAKEFLAGS" # Dynamic linker, see ld(1). ENV_ALLOWLIST="$ENV_ALLOWLIST LD_LIBRARY_PATH" # Locale, see locale(1). ENV_ALLOWLIST="$ENV_ALLOWLIST LANG LC_ALL LC_CTYPE LC_NUMERIC LC_TIME LC_COLLATE LC_MONETARY LC_MESSAGES LC_PAPER LC_NAME LC_ADDRESS LC_TELEPHONE LC_MEASUREMENT LC_IDENTIFICATION" ENV_ALLOWLIST="$ENV_ALLOWLIST SOURCE_DATE_EPOCH" for var in $ENV_ALLOWLIST do eval value="\$$var" # shellcheck disable=SC2154 test -n "$value" && printf ' %s=%q\n' "$var" "$value" >> "$BUILDINFO" done ``` replace “whitelist” with “allowlist” and properly quote variables with special characters (including newline) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#pullrequestreview-623199227___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
> So start by explaining what problem you're trying to solve, and not how other > people might've chosen to solve it. The purpose of having an automatically generated subpackage `buildinfo` is to have an exact description of the build environment used to obtain RPMs from a spec file. Having this, we can rebuild in the same condition any `src.rpm` (see my WIP tool called [rpmreproduce](https://github.com/fepitre/rpmreproduce/tree/devel-1.0-rpm) and compare with the original RPMs. Also, if a given package is supposed to be reproducible and rebuild differs, something is wrong and needs to be investigated. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#issuecomment-809310265___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
If you want feedback here, you should explain what it is and why we should care. Outside references are okay for additional background, but the what it does and why (as in rationale for the whole) needs to be explained right here so that it can be discussed. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#issuecomment-809299140___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
Just a little ping here to know if anyone has already take a look on the global form for this. @Conan-Kudo any feedback if you have time? Thank you very much. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532#issuecomment-782911313___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] Add support for generating buildinfo file as subpackage (#1532)
TODO: - Add Binary field with all built RPMs package names - Add Checksums for at least source RPM - Add additional rpm macros (--define, --with/--without, etc.) Reference: https://wiki.debian.org/ReproducibleBuilds/BuildinfoFiles, Thread Reproducible builds on Fedora devel list. You can view, comment on, or merge this pull request online at: https://github.com/rpm-software-management/rpm/pull/1532 -- Commit Summary -- * Add support for generating buildinfo file as subpackage -- File Changes -- M macros.in (28) A scripts/rpmbuildinfo (96) -- Patch Links -- https://github.com/rpm-software-management/rpm/pull/1532.patch https://github.com/rpm-software-management/rpm/pull/1532.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1532 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint