#85 Disable file fragment writing logic for SSH authorized_keys & share the spec between Fedora/CS9/RHEL
Merged 2 years ago by sohank2602. Opened 2 years ago by sohank2602.
rpms/ sohank2602/ignition rawhide  into  rawhide

file modified
+29 -1
@@ -1,5 +1,11 @@ 

  # Generated by go2rpm 1.3

+ %if 0%{?fedora}

  %bcond_without check

+ %else

+ # %gocheck isn't currently provided on CentOS/RHEL

+ # https://bugzilla.redhat.com/show_bug.cgi?id=1982298

+ %bcond_with check

+ %endif

  

  # https://github.com/coreos/ignition

  %global goipath         github.com/coreos/ignition
@@ -13,7 +19,7 @@ 

  %global dracutlibdir %{_prefix}/lib/dracut

  

  Name:           ignition

- Release:        1%{?dist}

+ Release:        2%{?dist}

  Summary:        First boot installer and configuration tool

  

  # Upstream license specification: Apache-2.0
@@ -211,6 +217,7 @@ 

  

  ############## validate-nonlinux subpackage ##############

  

+ %if 0%{?fedora}

  %package validate-nonlinux

  

  Summary:   Validation tool for Ignition configs for macOS and Windows
@@ -224,13 +231,22 @@ 

  through cross-compilation. Do not install it. It is only used for

  building binaries to sign by Fedora release engineering and include on the

  Ignition project's Github releases page.

+ %endif

  

  %prep

+ %if 0%{?fedora}

  %goprep -k

+ %else

+ %forgeautosetup -p1

+ %endif

  %autopatch -p1

  

  %build

  export LDFLAGS="-X github.com/coreos/ignition/v2/internal/version.Raw=%{version} -X github.com/coreos/ignition/v2/internal/distro.selinuxRelabel=true "

+ %if 0%{?rhel} || 0%{?centos}

+ # Need uncompressed debug symbols for debuginfo extraction

+ LDFLAGS+=' -X github.com/coreos/ignition/v2/internal/distro.writeAuthorizedKeysFragment=false -compressdwarf=false '

+ %endif

  export GOFLAGS="-mod=vendor"

  

  echo "Building ignition..."
@@ -241,11 +257,13 @@ 

  

  %global gocrossbuild go build -ldflags "${LDFLAGS:-} -B 0x$(head -c20 /dev/urandom|od -An -tx1|tr -d ' \\n')" -a -v -x

  

+ %if 0%{?fedora}

  echo "Building macOS ignition-validate..."

  GOARCH=amd64 GOOS=darwin %gocrossbuild -o ./ignition-validate-x86_64-apple-darwin validate/main.go

  

  echo "Building Windows ignition-validate..."

  GOARCH=amd64 GOOS=windows %gocrossbuild -o ./ignition-validate-x86_64-pc-windows-gnu.exe validate/main.go

+ %endif

  

  %install

  # dracut modules
@@ -256,9 +274,11 @@ 

  install -d -p %{buildroot}%{_bindir}

  install -p -m 0755 ./ignition-validate %{buildroot}%{_bindir}

  

+ %if 0%{?fedora}

  install -d -p %{buildroot}%{_datadir}/ignition

  install -p -m 0644 ./ignition-validate-x86_64-apple-darwin %{buildroot}%{_datadir}/ignition

  install -p -m 0644 ./ignition-validate-x86_64-pc-windows-gnu.exe %{buildroot}%{_datadir}/ignition

+ %endif

  

  # The ignition binary is only for dracut, and is dangerous to run from

  # the command line.  Install directly into the dracut module dir.
@@ -280,13 +300,21 @@ 

  %license %{golicenses}

  %{_bindir}/ignition-validate

  

+ %if 0%{?fedora}

  %files validate-nonlinux

  %license %{golicenses}

  %dir %{_datadir}/ignition

  %{_datadir}/ignition/ignition-validate-x86_64-apple-darwin

  %{_datadir}/ignition/ignition-validate-x86_64-pc-windows-gnu.exe

+ %endif

  

  %changelog

+ * Thu Aug 26 2021 Sohan Kunkerkar <skunkerk@redhat.com> - 2.12.0-2

+ - Disable file fragment writing logic for SSH authorized_keys on RHEL/CentOS

+ - Disable compressdwarf flag to avoid build failures on RHEL/CentOS

+ - Disable cross-building of Ignition-validate on RHEL/CentOS

+ - Conditionalize Fedora-specific configuration

+ 

  * Fri Aug 6 2021 Sohan Kunkerkar <skunkerk@redhat.com> - 2.12.0-1

  - New release

  

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

Any particular reason you're changing those lines instead of adding the condition around https://src.fedoraproject.org/rpms/ignition/blob/rawhide/f/ignition.spec#_233?

Any particular reason you're changing those lines instead of adding the condition around https://src.fedoraproject.org/rpms/ignition/blob/rawhide/f/ignition.spec#_233?

I think we're using that variable for both RHEL and Fedora. The writeAuthorizedKeys fragment should be disabled for RHEL, so thought of adding that line explicitly in the if condition.

But here you are changing the gocrossbuild command as well. I've not followed Ignition packaging but is this still needed for CentOS Stream 9 / RHEL 9? Also make sure to mention CentOS Stream as this not only for RHEL.

It looks like you added a .src.rpm (empty file) in this commit, that should be removed.

rebased onto b7043333f5d7e48da29c9c59c326eea1fa32783b

2 years ago

I think we're using that variable for both RHEL and Fedora.

Right, but I think he's suggesting:

%if 0%{?rhel} 
LDFLAGS+=' -X github.com/coreos/ignition/v2/internal/distro.writeAuthorizedKeysFragment=false'
%endif

right?

rebased onto 19e8eddd0eb89a76cc39ea110cb635443c5e28b9

2 years ago

rebased onto 651990e9d55fef89f27a53f9497c98dd5043f2ff

2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

rebased onto 4ad9a1f54227b562e4e9b6ee7c00ff93ca8f8a67

2 years ago

rebased onto 98b5253496d2ac7ed684604467ed87fbc3dc9240

2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

rebased onto ae0ea3974dc8353f923560d7e58b4cc4236cb3ca

2 years ago

rebased onto 9092c540120574ac8d6e636d002d103585f606f6

2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

rebased onto a0f1276f5288f03cd0c295680706a7128ea899f2

2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

OK, this looks better. Could you also mention the macOS/win binary disablement in the commit/changelog?

rebased onto e09b8fd97ce973058cd2dd0247802863ca3d9c74

2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

Minor/optional: would be good to track down the history behind that -compressdwarf=false and add a comment for it.

ignition-validate

Shouldn't this be "in RHEL/CentOS" instead of "for nonlinux distros"?

rebased onto fc99ea5de1db21fc067056dcf923ca264f6802c3

2 years ago

They are several changes all included in a single commit. Can you make a separated commit for each one:

  • the prefix -> lib macro change
  • the condition on fedora for windows/macos builds
  • the ssh key fragment change for C9S/RHEL
  • the dwarf comp change (is this is required for C9S? We should check that)

The main goal is to be able to copy/paste this specfile to C9S and have it work with no change so you should test that at the same time (maybe best to open a PR there for CI).

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

1 new commit added

  • Disable file fragment writing logic for SSH authorized_keys
2 years ago

rebased onto 03649a1e6b1cd8f3c745a5a99d29d5232452ef6d

2 years ago

rebased onto 641491400a5e233144d810ab2b420d19ec226d16

2 years ago

rebased onto 6e07b05b1723f4efb3e324af6e1d40edf7cc93f8

2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

This'll be /usr/lib64 on x86_64. We should keep matching dracut: https://src.fedoraproject.org/rpms/dracut/blob/rawhide/f/dracut.spec#_1

This'll be /usr/lib64 on x86_64. We should keep matching dracut: https://src.fedoraproject.org/rpms/dracut/blob/rawhide/f/dracut.spec#_1

I think it fails the rpmlint test

$ rpmlint ignition.spec 
ignition.spec:13: E: hardcoded-library-path in %{_prefix}/lib/dracut
0 packages and 1 specfiles checked; 1 errors, 0 warnings.

Specifically, rpm-linter in Zuul

rebased onto 114836138fa862a39441444d4316c7ad05bc1edd

2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

We still need this line though, right? We still want to define the version string, and enable SELinux relabeling.

Hmm I don't see anything in the dracut dist-git repo indicating they squashed that rpmlint warning. So I'd say we can just ignore the error. It's not new to this PR anyway.

Need to either bump release tag if you're planning to build right away, or omit the versioning on this entry.

rebased onto 20eb55a

2 years ago

Build succeeded.

Don't we need to also conditionalize on where we define the ignition-validate-nonlinux subpackage?

4 new commits added

  • Conditionalize Fedora-specific configuration
  • Disable cross-building of Ignition-validate on RHEL/CentOS
  • Disable compressdwarf flag to avoid build failures for RHEL/CentOS
  • Disable file fragment writing logic for SSH authorized_keys on RHEL/CentOS
2 years ago

4 new commits added

  • Conditionalize Fedora-specific configuration
  • Disable cross-building of Ignition-validate on RHEL/CentOS
  • Disable compressdwarf flag to avoid build failures for RHEL/CentOS
  • Disable file fragment writing logic for SSH authorized_keys on RHEL/CentOS
2 years ago

Build succeeded.

OK, this looks much better now. Will give it a final review ASAP.

4 new commits added

  • Conditionalize Fedora-specific configuration
  • Disable cross-building of Ignition-validate on RHEL/CentOS
  • Disable compressdwarf flag to avoid build failures for RHEL/CentOS
  • Disable file fragment writing logic for SSH authorized_keys on RHEL/CentOS
2 years ago

Build succeeded.

Why not just conditionalize %goprep?

I think a more canonical way to do this is to flip the bcond_without at the top to a bcond_with in the RHEL/CentOS case. Should also have a comment there which mentions that %gocheck isn't currently provided on CentOS/RHEL and link to https://bugzilla.redhat.com/show_bug.cgi?id=1982298 and/or https://gitlab.com/redhat/centos-stream/rpms/go-rpm-macros/-/blob/c9s/remove-fedora-dependency-automation.patch.

4 new commits added

  • Conditionalize Fedora-specific configuration
  • Disable cross-building of Ignition-validate on RHEL/CentOS
  • Disable compressdwarf flag to avoid build failures for RHEL/CentOS
  • Disable file fragment writing logic for SSH authorized_keys on RHEL/CentOS
2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

4 new commits added

  • Conditionalize Fedora-specific configuration
  • Disable cross-building of Ignition-validate on RHEL/CentOS
  • Disable compressdwarf flag to avoid build failures for RHEL/CentOS
  • Disable file fragment writing logic for SSH authorized_keys on RHEL/CentOS
2 years ago

Build succeeded.

What's %forgeautosetup? Or is this a typo?

I think we don't need an %else block at all here, no?

Might as well mention unit testing change here too if we're listing everything else.

Minor final comments, LGTM otherwise!

What's %forgeautosetup? Or is this a typo?

https://fedoraproject.org/wiki/More_Go_packaging#Usual_case

I think we don't need an %else block at all here, no?

That else block was introduced for rhel and centos

What's %forgeautosetup? Or is this a typo?

https://fedoraproject.org/wiki/More_Go_packaging#Usual_case

Ahh interesting. Hadn't seen it before.

In that case, LGTM. Feel free to ignore the changelog comment.

Nit: I think install -d -p %{buildroot}%{_datadir}/ignition should also be conditionalized for clarity, though the empty dir won't affect the resulting package.

Nit: disabling the compressdwarf flag could use a better comment, e.g. "Need uncompressed debug symbols for debuginfo extraction".

LGTM otherwise.

4 new commits added

  • Conditionalize Fedora-specific configuration
  • Disable cross-building of Ignition-validate on RHEL/CentOS
  • Need uncompressed debug symbols for debuginfo extraction RHEL/CentOS
  • Disable file fragment writing logic for SSH authorized_keys on RHEL/CentOS
2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

Build succeeded.

4 new commits added

  • Conditionalize Fedora-specific configuration
  • Disable cross-building of Ignition-validate on RHEL/CentOS
  • Need uncompressed debug symbols for debuginfo extraction on RHEL/CentOS
  • Disable file fragment writing logic for SSH authorized_keys on RHEL/CentOS
2 years ago

Build succeeded.

4 new commits added

  • Conditionalize Fedora-specific configuration
  • Disable cross-building of Ignition-validate on RHEL/CentOS
  • Disable compressdwarf flag to avoid build failures on RHEL/CentOS
  • Disable file fragment writing logic for SSH authorized_keys on RHEL/CentOS
2 years ago

Build succeeded.

Pull-Request has been merged by sohank2602

2 years ago
Metadata