Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preventing nested zipping of the portable Windows build #7950

Merged
merged 16 commits into from
Jan 23, 2025

Conversation

discip
Copy link
Contributor

@discip discip commented Jan 7, 2025

As stated in the header.

Makes extracting the actually required files a bit easier. 😊

@Noisyfox
Copy link
Collaborator

Is this still needed? Recent releases and nightly builds do not have nested zip files I believe.

@discip
Copy link
Contributor Author

discip commented Jan 11, 2025

Will check later,
not near my machine right now.

@discip
Copy link
Contributor Author

discip commented Jan 13, 2025

@Noisyfox
Good morning Sir,
as promised, I checked and found that this is still required as even the latest portable Windows build (https://github.com/SoftFever/OrcaSlicer/actions/runs/12734430763) is a nested zip file.

@discip
Copy link
Contributor Author

discip commented Jan 15, 2025

@Noisyfox
Just found the following comment regarding this topic, stating:

Ok. I grabbed the portable thinking it would keep me from screwing up my working installation.
Its a zip of a folder whose only content is a zip? That's odd.

Originally posted by @tlhintoq in #8038 (comment)

@Noisyfox
Copy link
Collaborator

@Noisyfox Just found the following comment regarding this topic, stating:

Ok. I grabbed the portable thinking it would keep me from screwing up my working installation.
Its a zip of a folder whose only content is a zip? That's odd.

Originally posted by @tlhintoq in #8038 (comment)

That's PR build, which is uploaded by github's own action actions/upload-artifact, I was referring to the nightly release which is uploaded by WebFreak001/deploy-nightly

@discip
Copy link
Contributor Author

discip commented Jan 15, 2025

That's PR build, which is uploaded by github's own action actions/upload-artifact, I was referring to the nightly release which is uploaded by WebFreak001/deploy-nightly

Ah!
That one works as expected.

But I think most tinkerers rely on the dev builds as you want to test a specific PR rather than the nightly build and if you don't want to compromise your current installation you need to go with the portabel version.
So it would be very nice if the portable version wasn't zipped twice. 😊

I hope that makes sense.

The changes I made are not state of the art, but that's all I was able to achieve.
Maybe you have a cleaner solution. I would highly appreciate it and even prefer it to mine.

.github/workflows/build_orca.yml Outdated Show resolved Hide resolved
.github/workflows/build_orca.yml Outdated Show resolved Hide resolved
@discip discip requested a review from Noisyfox January 23, 2025 06:26
# if: inputs.os == 'windows-latest'
# working-directory: ${{ github.workspace }}/build
# shell: cmd
# run: '"C:/Program Files/7-Zip/7z.exe" a -tzip OrcaSlicer_Windows_${{ env.ver }}_portable.zip ${{ github.workspace }}/build/OrcaSlicer'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you have to uncomment these part otherwise the zip file doesn't exist.

Copy link
Contributor Author

@discip discip Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the thing.
If this is uncommented the files get zipped the 1st time and than it gets zipped the second time with this:

      - name: Upload artifacts Win zip
        if: inputs.os == 'windows-latest'
        uses: actions/upload-artifact@v4
        with:
          name: OrcaSlicer_Windows_${{ env.ver }}_portable
          path: ${{ github.workspace }}/build/OrcaSlicer

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as Path points to the dir not the zip it should be fine.

@discip discip requested a review from Noisyfox January 23, 2025 07:56
@discip discip requested a review from Noisyfox January 23, 2025 09:24
Copy link
Collaborator

@Noisyfox Noisyfox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks~ I'll merge this once the PR build completed.

@Noisyfox Noisyfox merged commit 2518870 into SoftFever:main Jan 23, 2025
16 checks passed
@discip discip deleted the patch-3 branch January 23, 2025 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants