You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2022/07/22 19:41:26 UTC

[GitHub] [daffodil] tuxji commented on a diff in pull request #816: Improve release candidate container build cachability and reproducability

tuxji commented on code in PR #816:
URL: https://github.com/apache/daffodil/pull/816#discussion_r927932971


##########
containers/release-candidate/Dockerfile:
##########
@@ -15,20 +15,61 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-FROM registry.fedoraproject.org/fedora-minimal:36
+FROM registry.fedoraproject.org/fedora:36
 
 WORKDIR /root
 
-COPY setup-container.sh /usr/bin/
-RUN /usr/bin/setup-container.sh
+# Delete cruft fedora leaves in the homedir
+RUN rm /root/*

Review Comment:
   This command will return an error status if /root contains any visible (not hidden) directories.  If you run `rm -rf /root/*` instead, it will not return any error status.



##########
containers/release-candidate/Dockerfile:
##########
@@ -15,20 +15,61 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-FROM registry.fedoraproject.org/fedora-minimal:36
+FROM registry.fedoraproject.org/fedora:36
 
 WORKDIR /root
 
-COPY setup-container.sh /usr/bin/
-RUN /usr/bin/setup-container.sh
+# Delete cruft fedora leaves in the homedir
+RUN rm /root/*
 
-# Needed to get WiX to run in wine on Linux. See wix_wine.sh for more details
-# on why we need to do this and how it works
+# Install dependencies, explicitly not enabling the updates repo so we are
+# pinned at a particular fedora release.

Review Comment:
   We risk building a Fedora container with a CVE vulnerability fixed by the updates repo.  I'm not sure disabling the updates repo is worth that risk, although we would need to run `dnf upgrade` as well to apply all the patches correctly.



##########
containers/release-candidate/Dockerfile:
##########
@@ -15,20 +15,61 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-FROM registry.fedoraproject.org/fedora-minimal:36
+FROM registry.fedoraproject.org/fedora:36
 
 WORKDIR /root
 
-COPY setup-container.sh /usr/bin/
-RUN /usr/bin/setup-container.sh
+# Delete cruft fedora leaves in the homedir
+RUN rm /root/*
 
-# Needed to get WiX to run in wine on Linux. See wix_wine.sh for more details
-# on why we need to do this and how it works
+# Install dependencies, explicitly not enabling the updates repo so we are
+# pinned at a particular fedora release.
+RUN \
+  dnf -y --quiet --repo=fedora install \
+    clang \
+    git \
+    java-1.8.0-devel \
+    llvm \
+    mxml-devel \
+    npm \
+    pinentry \
+    rpm-build \
+    rpm-sign \
+    subversion \
+    unzip \
+    vim-minimal \
+    wine \
+    winetricks && \
+  dnf clean all
+
+# Enable SBT pgp plugin
+COPY src/plugins.sbt /root/.sbt/1.0/plugins/
+
+# Install wix, including changes to allow WiX to run in wine on Linux. See
+# src/wix_wine.sh for more details on why we need to do this and how it works
+RUN \
+  curl -sS -L https://github.com/wixtoolset/wix3/releases/download/wix3112rtm/wix311-binaries.zip -o wix311-binaries.zip && \
+  echo "6fd961c85e1e6adafb99ef50c9659e3eb83c84ecaf49f523e556788845b206e1857aba2c39409405d4cda1df9b30a552ce5aab808be5f8368a37a447d78d1a05 wix311-binaries.zip" | sha512sum --quiet -c - && \
+  mkdir /opt/wix311 && \
+  unzip -q wix311-binaries.zip -d /opt/wix311/ && \
+  rm wix311-binaries.zip
 RUN mv /opt/wix311/{candle.exe,real-candle.exe}
 RUN mv /opt/wix311/{light.exe,real-light.exe}
-COPY wix_wine.sh /opt/wix311/candle.exe
-COPY wix_wine.sh /opt/wix311/light.exe
+COPY src/wix_wine.sh /opt/wix311/candle.exe
+COPY src/wix_wine.sh /opt/wix311/light.exe
+
+# Install a pinned version of sbt, needed because the spt-rpm repository
+# frequently adds new versions.
+RUN \
+  dnf -y --quiet --repofrompath=sbt-rpm,https://repo.scala-sbt.org/scalasbt/rpm --repo=sbt-rpm --nogpgcheck install \
+    sbt-1.7.1 && \

Review Comment:
   When and how do you propose we bump these pinned versions of sbt and yarn?  Add a bullet item to the release checklist to bump sbt and yarn to their latest versions whenever we make a new release?



##########
containers/release-candidate/Dockerfile:
##########
@@ -15,20 +15,61 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-FROM registry.fedoraproject.org/fedora-minimal:36
+FROM registry.fedoraproject.org/fedora:36
 
 WORKDIR /root
 
-COPY setup-container.sh /usr/bin/
-RUN /usr/bin/setup-container.sh
+# Delete cruft fedora leaves in the homedir
+RUN rm /root/*
 
-# Needed to get WiX to run in wine on Linux. See wix_wine.sh for more details
-# on why we need to do this and how it works
+# Install dependencies, explicitly not enabling the updates repo so we are
+# pinned at a particular fedora release.
+RUN \
+  dnf -y --quiet --repo=fedora install \
+    clang \
+    git \
+    java-1.8.0-devel \

Review Comment:
   Java 8 is pretty close to end of life.  Would Java 11 work here?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org