Cleanup old branch builds (slic3r#4680)

* slic3r#4620 Invalidate posDetectSurfaces during make_perimeters() when posSlice would also get invalidated. Can't use invalidate_step() as that invalidates the perimeters as well. * Add a test step to check for regression slic3r#4620 - behavior on running Print::process() again on the same model. * Fix naming scheme for macos/osx builds to make it obvious if it is a PR or not. * Remove previous versions of the same kind of branch or PR ID * Adjust copied statements to account for different calling conventions; osx does not have an arch name argument for make_dmg * Remember to set the version too * Update windows build server command to clean up old branch builds. * Specifically look for PR${PR_ID} instead of just ${PR_ID} to avoid unintentional deletions. * don't symlink pull requests * Update to call more functions on osx * Only delete/clean once, not once per file uploaded.
klocs · Jan 6, 2019 · b3cac9d · b3cac9d
1 parent 748dd3c
commit b3cac9d
Show file tree

Hide file tree

Showing 7 changed files with 108 additions and 14 deletions.
diff --git a/package/deploy/sftp-symlink.sh b/package/deploy/sftp-symlink.sh
@@ -21,6 +21,9 @@ fi
 source $(dirname $0)/../common/util.sh
 set_pr_id
 set_branch
+if [ ! -z ${PR_ID+x} ]; then
+    exit 0
+fi
 if [ ! -z ${PR_ID+x} ] || [ $current_branch != "master" ]; then
     DIR=${DIR}/branches
 fi

diff --git a/package/deploy/sftp.sh b/package/deploy/sftp.sh
@@ -17,18 +17,28 @@ if [ ! -z ${PR_ID+x} ] || [ $current_branch != "master" ]; then
 fi
 
 if [ -s $KEY ]; then
-    for i in $FILES; do 
-         filepath=$i  # this is expected to be an absolute path
-         tmpfile=$(mktemp)
-         echo put $filepath > $tmpfile 
-         sftp -b $tmpfile -i$KEY "${UPLOAD_USER}@dl.slic3r.org:$DIR/"
-         result=$?
-         if [ $? -eq 1 ]; then 
-             echo "Error with SFTP"
-             exit $result; 
-         fi
+    if [ ! -z ${PR_ID+x} ] || [ $current_branch != "master" ]; then
+        # clean up old copies of the same branch/PR
+        if [ ! -z ${PR_ID+x} ]; then
+            echo "rm *PR${PR_ID}*" | sftp -i$KEY "${UPLOAD_USER}@dl.slic3r.org:$DIR/"
+        fi
+        if [ $current_branch != "master" ]; then
+            echo "rm *${current_branch}*" | sftp -i$KEY "${UPLOAD_USER}@dl.slic3r.org:$DIR/"
+        fi
+    fi
+    for i in $FILES; do
+        filepath=$i  # this is expected to be an absolute path
+        tmpfile=$(mktemp)
+        echo put $filepath > $tmpfile
+
+        sftp -b $tmpfile -i$KEY "${UPLOAD_USER}@dl.slic3r.org:$DIR/"
+        result=$?
+        if [ $? -eq 1 ]; then
+            echo "Error with SFTP"
+            exit $result;
+        fi
     done
 else
-    echo "$KEY is not available, not deploying." 
+    echo "$KEY is not available, not deploying."
 fi
 exit $result
diff --git a/package/deploy/winscp.ps1 b/package/deploy/winscp.ps1
@@ -12,6 +12,12 @@ Set-Variable -Name "UUSER" -Value "$env:UPLOAD_USER"
 Set-Variable -Name "UPLOAD" -Value "$($FILE | Resolve-Path)"
 if (Test-Path $KEY) {
     if ($env:APPVEYOR_PULL_REQUEST_NUMBER -Or $env:APPVEYOR_REPO_BRANCH -ne "master" ) {
+        if ($env:APPVEYOR_PULL_REQUEST_NUMBER) {
+            winscp.com /privatekey=$KEY /command "open sftp://$UUSER@dl.slic3r.org/$DIR/branches -hostkey=*" "rm *PR${env:APPVEYOR_PULL_REQUEST_NUMBER}*" "exit"
+        }
+        if ($env:APPVEYOR_REPO_BRANCH -ne "master" ) {
+            winscp.com /privatekey=$KEY /command "open sftp://$UUSER@dl.slic3r.org/$DIR/branches -hostkey=*" "rm *${env:APPVEYOR_REPO_BRANCH}*" "exit"
+        }
         winscp.com /privatekey=$KEY /command "open sftp://$UUSER@dl.slic3r.org/$DIR/branches -hostkey=*" "put $UPLOAD ./$FILE" "exit"
     } else {
         winscp.com /privatekey=$KEY /command "open sftp://$UUSER@dl.slic3r.org/$DIR -hostkey=*" "put $UPLOAD ./$FILE" "exit"

diff --git a/package/osx/make_dmg.sh b/package/osx/make_dmg.sh
@@ -10,6 +10,21 @@ set -euo pipefail
 # While we might have a pp executable in our path, it might not be
 # using the perl binary we have in path, so make sure they belong
 # to the same Perl instance:
+
+source $(dirname $0)/../common/util.sh
+if [ $# -lt 1 ]; then
+    set_source_dir
+else
+    set_source_dir $1
+fi
+set_version
+get_commit
+set_build_id
+set_branch
+set_app_name
+set_pr_id
+
+
 if !(perl -Mlocal::lib=local-lib -MPAR::Packer -e1 2> /dev/null); then
     echo "The PAR::Packer module was not found; installing..."
     cpanm --local-lib local-lib PAR::Packer
@@ -49,7 +64,16 @@ else
     fi
 fi
 
-dmgfile=slic3r-${SLIC3R_BUILD_ID}.dmg
+if [ "$current_branch" == "master" ]; then
+    if [ ! -z ${PR_ID+x} ]; then
+        dmgfile=slic3r-${SLIC3R_BUILD_ID}-osx-PR${PR_ID}.dmg
+    else
+        dmgfile=slic3r-${SLIC3R_BUILD_ID}-osx.dmg
+    fi
+else
+    dmgfile=slic3r-${SLIC3R_BUILD_ID}-osx-${current_branch}.dmg
+fi
+
 echo "DMG filename: ${dmgfile}"
 
 rm -rf $WD/_tmp

diff --git a/package/win/package_win32.ps1 b/package/win/package_win32.ps1
@@ -59,7 +59,11 @@ if (!($arch -eq "64bit" -Or $arch -eq "32bit")) {
 }
 
 if ($env:APPVEYOR) {
-    $output_zip = "${scriptDir}\..\..\Slic3r-${branch}.${currentDate}.${env:APPVEYOR_BUILD_NUMBER}.$(git rev-parse --short HEAD).${arch}.zip"
+    if ($env:APPVEYOR_PULL_REQUEST_NUMBER -eq "") {
+        $output_zip = "${scriptDir}\..\..\Slic3r-${branch}.${currentDate}.${env:APPVEYOR_BUILD_NUMBER}.$(git rev-parse --short HEAD).${arch}.zip"
+    } else {
+        $output_zip = "${scriptDir}\..\..\Slic3r-${branch}.${currentDate}.${env:APPVEYOR_BUILD_NUMBER}.$(git rev-parse --short HEAD).${arch}-PR${APPVEYOR_PULL_REQUEST_NUMBER}.zip"
+    }
 } else {
     $output_zip = "${scriptDir}\..\..\Slic3r-${branch}.${currentDate}.$(git rev-parse --short HEAD).${arch}.zip"
 }

diff --git a/src/test/libslic3r/test_print.cpp b/src/test/libslic3r/test_print.cpp
@@ -60,6 +60,50 @@ SCENARIO("Print: Skirt generation") {
     }
 }
 
+void test_is_solid_infill(shared_Print p, size_t obj_id, size_t layer_id ) {
+    const PrintObject& obj { *(p->objects.at(obj_id)) };
+    const Layer& layer { *(obj.get_layer(layer_id)) };
+
+    // iterate over all of the regions in the layer
+    for (const LayerRegion* reg : layer.regions) {
+        // for each region, iterate over the fill surfaces
+        for (const Surface& s : reg->fill_surfaces) {
+            CHECK(s.is_solid());
+        }
+    }
+}
+
+SCENARIO("Print: Changing number of solid surfaces does not cause all surfaces to become internal.") {
+    GIVEN("sliced 20mm cube and config with top_solid_surfaces = 2 and bottom_solid_surfaces = 1") {
+        auto config {Slic3r::Config::new_from_defaults()};
+        TestMesh m { TestMesh::cube_20x20x20 };
+        config->set("top_solid_layers", 2);
+        config->set("bottom_solid_layers", 1);
+        config->set("layer_height", 0.5); // get a known number of layers
+        config->set("first_layer_height", 0.5);
+        Slic3r::Model model;
+        auto event_counter {0U};
+        std::string stage;
+        auto print {Slic3r::Test::init_print({m}, model, config)};
+        print->process();
+        test_is_solid_infill(print, 0, 39); // check to make sure it sliced properly
+        WHEN("Model is re-sliced with top_solid_layers == 3") {
+            print->regions[0]->config.top_solid_layers = 3;
+            print->objects[0]->invalidate_step(posPrepareInfill);
+            print->process();
+            THEN("Print object does not have 0 bottom surfaces.") {
+                test_is_solid_infill(print, 0, 0);
+            }
+            AND_THEN("Print object has 3 top solid layers") {
+                test_is_solid_infill(print, 0, 39);
+                test_is_solid_infill(print, 0, 38);
+                test_is_solid_infill(print, 0, 37);
+            }
+        }
+    }
+
+}
+
 SCENARIO("Print: Brim generation") {
     GIVEN("20mm cube and default config, 1mm first layer width") {
         auto config {Slic3r::Config::new_from_defaults()};

diff --git a/xs/src/libslic3r/PrintObject.cpp b/xs/src/libslic3r/PrintObject.cpp
@@ -1060,8 +1060,11 @@ PrintObject::make_perimeters()
     // Temporary workaround for detect_surfaces_type() not being idempotent (see #3764).
     // We can remove this when idempotence is restored. This make_perimeters() method
     // will just call merge_slices() to undo the typed slices and invalidate posDetectSurfaces.
-    if (this->typed_slices)
+    if (this->typed_slices) {
         this->state.invalidate(posSlice);
+        // also invalidate posDetectSurfaces
+        this->state.invalidate(posDetectSurfaces);
+    }
 
     // prerequisites
     this->slice();