use Files.createTempFile (#141)

* use Files.createTempFile * Don't always apply POSIX permissions * back out permissions change * Revert "back out permissions change" This reverts commit 8ec486b. * pin back windows * add slack alert & cron * broken dockcross tag * docker rmi * build only specific artifact per test * fix
uber · Jun 21, 2024 · cfb9b61 · cfb9b61
1 parent 0dfcd64
commit cfb9b61
Show file tree

Hide file tree

Showing 6 changed files with 87 additions and 18 deletions.
diff --git a/.github/workflows/slack-alert.yml b/.github/workflows/slack-alert.yml
@@ -0,0 +1,22 @@
+name: slack-alert
+
+on:
+  workflow_run:
+    workflows: [tests]
+    types: [completed]
+
+jobs:
+  on-failure:
+    runs-on: ubuntu-latest
+    if: ${{ github.event.workflow_run.conclusion == 'failure' }}
+    steps:
+      - name: Send Slack Alert
+        id: slack
+        uses: slackapi/[email protected]
+        with:
+          payload: |
+            Github Actions ${{ github.event.workflow_run.conclusion }}
+            Repo: ${{github.event.workflow_run.repository.name }}
+            Workflow URL: ${{ github.event.workflow_run.html_url }}
+        env:
+          SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }}
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -5,6 +5,9 @@ on:
     branches: [master, stable-*]
   pull_request:
     branches: [master, stable-*]
+  schedule:
+    # Every Sunday, rerun
+    - cron: "0 12 * * 0"
 
 jobs:
   tests:
@@ -56,15 +59,16 @@ jobs:
           if-no-files-found: error 
 
   tests-new-dockcross:
-    name: Dockcross ${{ matrix.dockcross-tag }} Java ${{ matrix.java-version }} ${{ matrix.os }}
+    name: Dockcross ${{ matrix.dockcross-tag }} Java ${{ matrix.java-version }} ${{ matrix.os }} ${{ matrix.dockcross-only }}
     runs-on: ${{ matrix.os }}
 
     strategy:
       matrix:
         os: [ubuntu-latest]
         java-distribution: [adopt]
         java-version: [17]
-        dockcross-tag: ["20220906-e88a3ce", "20230116-670f7f7"]
+        dockcross-tag: ["20230116-670f7f7", "20240418-88c04a4", "latest"]
+        dockcross-only: ["android-arm", "android-arm64", "linux-arm64", "linux-armv5", "linux-armv7", "linux-s390x", "linux-ppc64le", "linux-x64", "linux-x86", "windows-static-x64", "windows-static-x86"]
 
     steps:
       - uses: actions/[email protected]
@@ -85,7 +89,7 @@ jobs:
             ${{ runner.os }}-maven-
 
       - name: Tests
-        run: mvn "-Dh3.system.prune=true" "-Dh3.dockcross.tag=${{ matrix.dockcross-tag }}" -B -V clean test
+        run: mvn "-Dh3.system.prune=true" "-Dh3.dockcross.tag=${{ matrix.dockcross-tag }}" "-Dh3.dockcross.only=${{ matrix.dockcross-only }}" -B -V clean test
 
   tests-no-docker:
     name: Java (No Docker) ${{ matrix.java-version }} ${{ matrix.os }}
@@ -94,7 +98,8 @@ jobs:
     strategy:
       matrix:
         # TODO: Docker on macos-latest running is not working
-        os: [macos-latest, windows-latest]
+        # TODO: Windows pinned back
+        os: [macos-latest, windows-2019]
         java-distribution: [adopt]
         java-version: [8, 11, 15, 17]
 

diff --git a/pom.xml b/pom.xml
@@ -72,6 +72,7 @@
         <h3.use.docker>true</h3.use.docker>
         <h3.system.prune>false</h3.system.prune>
         <h3.dockcross.tag>20210624-de7b1b0</h3.dockcross.tag>
+        <h3.dockcross.only />
         <h3.github.artifacts.use>false</h3.github.artifacts.use>
         <h3.github.artifacts.by_run />
         <!-- Used for passing additional arguments to surefire when using JaCoCo -->
@@ -270,6 +271,7 @@
                                 <argument>${h3.use.docker}</argument>
                                 <argument>${h3.system.prune}</argument>
                                 <argument>${h3.dockcross.tag}</argument>
+                                <argument>${h3.dockcross.only}</argument>
                                 <argument>${h3.github.artifacts.use}</argument>
                                 <argument>${h3.github.artifacts.by_run}</argument>
                             </arguments>

diff --git a/src/main/c/h3-java/build-h3.sh b/src/main/c/h3-java/build-h3.sh
@@ -25,6 +25,7 @@
 #                    system prune will be run after each step
 #                    (i.e. for disk space constrained environments like CI)
 # dockcross-tag    - Tag name for dockcross
+# dockcross-only   - When set, build only a specific architecture with dockcross.
 # github-artifacts - When set, all build artifacts are retrieved from Github
 #                    Actions artifacts rather than built locally (overrides
 #                    all other settings.)
@@ -44,8 +45,9 @@ GIT_REVISION=$2
 USE_DOCKER=$3
 SYSTEM_PRUNE=$4
 DOCKCROSS_TAG=$5
-GITHUB_ARTIFACTS=$6
-GITHUB_ARTIFACTS_RUN=$7
+DOCKCROSS_ONLY=$6
+GITHUB_ARTIFACTS=$7
+GITHUB_ARTIFACTS_RUN=$8
 
 if $GITHUB_ARTIFACTS; then
     src/main/c/h3-java/pull-from-github.sh "$GITHUB_ARTIFACTS_RUN"
@@ -180,10 +182,15 @@ UPGRADE_CMAKE=true
 CMAKE_ROOT=target/cmake-3.23.2-linux-x86_64
 mkdir -p $CMAKE_ROOT
 
+DOCKCROSS_IMAGES="android-arm android-arm64 linux-arm64 linux-armv5 linux-armv7 linux-s390x linux-ppc64le linux-x64 linux-x86 windows-static-x64 windows-static-x86"
+if ! $DOCKCROSS_ONLY; then
+    DOCKCROSS_IMAGES=$DOCKCROSS_ONLY
+    echo Building only: $DOCKCROSS_IMAGES
+fi
+
 # linux-armv6 excluded because of build failure
 # linux-mips excluded due to manifest error
-for image in android-arm android-arm64 linux-arm64 linux-armv5 linux-armv7 linux-s390x \
-    linux-ppc64le linux-x64 linux-x86 windows-static-x64 windows-static-x86; do
+for image in $DOCKCROSS_IMAGES; do
 
     # Setup for using dockcross
     BUILD_ROOT=target/h3-java-build-$image
@@ -210,6 +217,9 @@ for image in android-arm android-arm64 linux-arm64 linux-armv5 linux-armv7 linux
     if [ -e $BUILD_ROOT/lib/libh3-java.dll ]; then cp $BUILD_ROOT/lib/libh3-java.dll $OUTPUT_ROOT ; fi
 
     if $SYSTEM_PRUNE; then
+        # Remove the image we just ran
+        docker rmi dockcross/$image:$DOCKCROSS_TAG
+        # Aggressively try to free more disk space
         docker system prune --force --all
         docker system df
         rm $BUILD_ROOT/dockcross

diff --git a/src/main/java/com/uber/h3core/H3CoreLoader.java b/src/main/java/com/uber/h3core/H3CoreLoader.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2017-2018 Uber Technologies, Inc.
+ * Copyright 2017-2018, 2024 Uber Technologies, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,7 +20,12 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.nio.file.Files;
+import java.nio.file.attribute.FileAttribute;
+import java.nio.file.attribute.PosixFilePermission;
+import java.nio.file.attribute.PosixFilePermissions;
 import java.util.Locale;
+import java.util.Set;
 
 /** Extracts the native H3 core library to the local filesystem and loads it. */
 public final class H3CoreLoader {
@@ -60,11 +65,6 @@ private static void copyStream(InputStream in, OutputStream out) throws IOExcept
    * @throws UnsatisfiedLinkError The resource path does not exist
    */
   static void copyResource(String resourcePath, File newH3LibFile) throws IOException {
-    // Set the permissions
-    newH3LibFile.setReadable(true);
-    newH3LibFile.setWritable(true, true);
-    newH3LibFile.setExecutable(true, true);
-
     // Shove the resource into the file and close it
     try (InputStream resource = H3CoreLoader.class.getResourceAsStream(resourcePath)) {
       if (resource == null) {
@@ -93,6 +93,24 @@ public static NativeMethods loadNatives() throws IOException {
     return loadNatives(os, arch);
   }
 
+  private static File createTempLibraryFile(OperatingSystem os) throws IOException {
+    if (os.isPosix()) {
+      // Note this is already done by the implementation of Files.createTempFile that I looked at,
+      // but the javadoc does not seem to gaurantee the permissions will be restricted to owner
+      // write.
+      final FileAttribute<Set<PosixFilePermission>> attr =
+          PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rwx------"));
+      return Files.createTempFile("libh3-java", os.getSuffix(), attr).toFile();
+    } else {
+      // When not a POSIX OS, try to ensure the permissions are secure
+      final File f = Files.createTempFile("libh3-java", os.getSuffix()).toFile();
+      f.setReadable(true, true);
+      f.setWritable(true, true);
+      f.setExecutable(true, true);
+      return f;
+    }
+  }
+
   /**
    * For use when the H3 library should be unpacked from the JAR and loaded. The native library for
    * the specified operating system and architecture will be extract.
@@ -115,8 +133,7 @@ public static synchronized NativeMethods loadNatives(OperatingSystem os, String
       final String dirName = String.format("%s-%s", os.getDirName(), arch);
       final String libName = String.format("libh3-java%s", os.getSuffix());
 
-      final File newLibraryFile = File.createTempFile("libh3-java", os.getSuffix());
-
+      final File newLibraryFile = createTempLibraryFile(os);
       newLibraryFile.deleteOnExit();
 
       copyResource(String.format("/%s/%s", dirName, libName), newLibraryFile);
@@ -146,13 +163,20 @@ public enum OperatingSystem {
     ANDROID(".so"),
     DARWIN(".dylib"),
     FREEBSD(".so"),
-    WINDOWS(".dll"),
+    WINDOWS(".dll", false),
     LINUX(".so");
 
     private final String suffix;
+    private final boolean posix;
 
     OperatingSystem(String suffix) {
       this.suffix = suffix;
+      this.posix = true;
+    }
+
+    OperatingSystem(String suffix, boolean posix) {
+      this.suffix = suffix;
+      this.posix = posix;
     }
 
     /** Suffix for native libraries. */
@@ -164,6 +188,11 @@ public String getSuffix() {
     public String getDirName() {
       return name().toLowerCase(H3_CORE_LOCALE);
     }
+
+    /** Whether to try to use POSIX file permissions when creating the native library temp file. */
+    public boolean isPosix() {
+      return this.posix;
+    }
   }
 
   /**

diff --git a/src/test/java/com/uber/h3core/TestH3CoreLoader.java b/src/test/java/com/uber/h3core/TestH3CoreLoader.java
@@ -19,6 +19,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Files;
 import org.junit.Test;
 
 /** H3CoreLoader is mostly tested by {@link TestH3CoreFactory}. This also tests OS detection. */
@@ -60,7 +61,7 @@ public void testDetectArch() {
 
   @Test(expected = UnsatisfiedLinkError.class)
   public void testExtractNonexistant() throws IOException {
-    File tempFile = File.createTempFile("test-extract-resource", null);
+    File tempFile = Files.createTempFile("test-extract-resource", null).toFile();
 
     tempFile.deleteOnExit();