shadow-maint / shadow

Upstream shadow tree
Other
292 stars 228 forks source link

share/containers/, .github/workflows/: Don't make(1) twice #950

Closed alejandro-colomar closed 6 months ago

alejandro-colomar commented 6 months ago

It was being done so that the second one prints errors without races. However, the same thing can be done by passing -Orecurse to make(1).

And this makes the logs even more readable, since there's no racy output at all.

Fixes: 97f79e3b2715 ("CI: Make build logs more readable") Link: https://github.com/shadow-maint/shadow/pull/702 Link: https://github.com/nginx/unit/pull/1123 Acked-by: @ikerexxe Cc: @ac000 Cc: @thresheek Cc: @arbourd

alejandro-colomar commented 6 months ago

Since we've made the cut for 4.15, I'll change this to draft, until after the release.

alejandro-colomar commented 6 months ago

v1b changes:

$ git range-diff gh/master..gh/make shadow/master..make 
1:  2329d89c = 1:  4ed47e82 share/containers/, .github/workflows/: Don't make(1) twice
alejandro-colomar commented 6 months ago

v2 changes:

$ git range-diff gh/master..gh/make alx/master..make 
1:  4ed47e82 ! 1:  6e09d63b share/containers/, .github/workflows/: Don't make(1) twice
    @@ .github/workflows/static-code-analysis.yml: jobs:
            uses: github/codeql-action/analyze@v2

      ## share/containers/alpine.dockerfile ##
    -@@ share/containers/alpine.dockerfile: COPY ./ /usr/local/src/shadow/
    - WORKDIR /usr/local/src/shadow/
    - 
    - RUN ./autogen.sh --without-selinux --disable-man --disable-nls --with-yescrypt
    +@@ share/containers/alpine.dockerfile: RUN ./autogen.sh \
    +   --disable-man \
    +   --disable-nls \
    +   --with-yescrypt
     -RUN make -kj4 || true
     -RUN make
     +RUN make -Orecurse -j4
    - RUN make check
    + RUN bash -c "trap 'cat <tests/unit/test-suite.log >&2' ERR; make check;"
      RUN make install

      ## share/containers/debian.dockerfile ##
    -@@ share/containers/debian.dockerfile: COPY ./ /usr/local/src/shadow/
    - WORKDIR /usr/local/src/shadow/
    - 
    - RUN ./autogen.sh --without-selinux --enable-man --with-yescrypt
    +@@ share/containers/debian.dockerfile: RUN ./autogen.sh \
    +   --without-selinux \
    +   --enable-man \
    +   --with-yescrypt
     -RUN make -kj4 || true
     -RUN make
     +RUN make -Orecurse -j4
    - RUN make check
    + RUN bash -c "trap 'cat <tests/unit/test-suite.log >&2' ERR; make check;"
      RUN make install

      ## share/containers/fedora.dockerfile ##
    -@@ share/containers/fedora.dockerfile: RUN ./autogen.sh --enable-shadowgrp --enable-man --with-audit \
    -         --with-sha-crypt --with-bcrypt --with-yescrypt --with-selinux \
    -         --without-libpam --enable-shared --without-libbsd \
    -         --with-group-name-max-length=32 --enable-lastlog --enable-logind=no
    +@@ share/containers/fedora.dockerfile: RUN ./autogen.sh \
    +         --with-group-name-max-length=32 \
    +   --enable-lastlog \
    +   --enable-logind=no
     -RUN make -kj4 || true
     -RUN make
     +RUN make -Orecurse -j4
    - RUN make check
    + RUN bash -c "trap 'cat <tests/unit/test-suite.log >&2' ERR; make check;"
      RUN make install
alejandro-colomar commented 6 months ago

v2b changes:

$ git range-diff 89c4da43..gh/make  alx/master..make 
1:  6e09d63b = 1:  2509ca7e share/containers/, .github/workflows/: Don't make(1) twice