Skip to content

bug: 🐝 The clean-exclude option is not just for cleaning #1827

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

Open
koenbollen opened this issue Apr 1, 2025 · 6 comments
Open

bug: 🐝 The clean-exclude option is not just for cleaning #1827

koenbollen opened this issue Apr 1, 2025 · 6 comments
Assignees
Labels
bug 🐝 This issue describes a bug. triage ⚠️

Comments

@koenbollen
Copy link

koenbollen commented Apr 1, 2025

Describe the bug

When setting the clean-exclude option, I was expecting it to only count for a 'clean' step/phase. But there is only one step (rsync the --delete option is just added when clean:true).

The clean-exclude is just a general exclude. If other people exclude files, these files will also not be copied over.

I think this can be fixed with a slight change to the documentation.

Reproduction Steps

Context:
I wanted to deploy to a subfolder with the commit sha as a name for each commit.

Example:

deploy:
  name: Deploy
  needs: [build] # this generates an artifact
  runs-on: ubuntu-latest
  permissions:
    contents: write
  steps:
    - uses: actions/checkout@v4
      with:
        fetch-depth: 5
    - uses: actions/download-artifact@v4
      with:
        name: my-project-${{github.ref_name}}-${{github.sha}}
        path: dist/${{github.sha}}
    - name: Get Previous Commits
      run: |
        echo "LAST_FIVE_COMMITS<<EOF" >> $GITHUB_ENV
        git rev-list --max-count=5 "${{github.sha}}" >> $GITHUB_ENV
        echo "EOF" >> $GITHUB_ENV
    - uses: JamesIves/github-pages-deploy-action@6c2d9db40f9296374acc17b90404b6e8864128c8 # v4
      with:
        folder: dist
        clean-exclude: |
          index.html
          ${{ env.LAST_FIVE_COMMITS }}
        force: false

This only leaves the index.html. Because the ${{github.sha}} i want to deploy is also part of the clean-exclude list, it's never copied over by rsync.

Logs

/usr/bin/rsync -q -av --checksum --progress /home/runner/work/action-tester/action-tester/dist/. github-pages-deploy-action-temp-deployment-folder --delete --exclude index.html --exclude pr-preview/ --exclude 4521fe79967bbb5ce2b3fb13ebc3483375bfbd3f --exclude 001e1f753cc025ef2490ae9b12282b20ed890dfe --exclude 8deac04a32622e86ddc7b560de692a084411fd33 --exclude 22aece381e3fb637c634a1270768462dba48ef7c --exclude 96f1ebeef5b14832e012bd97a628db11ac2be5f4 --exclude CNAME --exclude .nojekyll --exclude .ssh --exclude .git --exclude .github
Checking if there are files to commit…
Running post deployment cleanup jobs… 🗑️

Workflow

deploy:
  name: Deploy
  needs: [build] # this generates an artifact
  runs-on: ubuntu-latest
  permissions:
    contents: write
  steps:
    - uses: actions/checkout@v4
      with:
        fetch-depth: 5
    - uses: actions/download-artifact@v4
      with:
        name: my-project-${{github.ref_name}}-${{github.sha}}
        path: dist/${{github.sha}}
    - name: Get Previous Commits
      run: |
        echo "LAST_FIVE_COMMITS<<EOF" >> $GITHUB_ENV
        git rev-list --max-count=5 "${{github.sha}}" >> $GITHUB_ENV
        echo "EOF" >> $GITHUB_ENV
    - uses: JamesIves/github-pages-deploy-action@6c2d9db40f9296374acc17b90404b6e8864128c8 # v4
      with:
        folder: dist
        clean-exclude: |
          index.html
          ${{ env.LAST_FIVE_COMMITS }}
        force: false

Additional Comments

btw, love this action! Thank you so much. Let me know if I can help with a pull request.

@koenbollen koenbollen added bug 🐝 This issue describes a bug. triage ⚠️ labels Apr 1, 2025
@JamesIves
Copy link
Owner

Happy to make some changes to the documentation! Do you have any suggestions?

@koenbollen
Copy link
Author

Trying to come up with a better description of clean-exclude I find it difficult to correct it.
--exclude is just not only for the deletion operations.

Two options:

  1. Change the option to be "exclude" (without the clean-), and the description to explain it will also exclude the files when copying over.
  2. Don't use the rsync option --exclude myfile.txt, but the option --filter "P myfile.txt". That does protect files from deletion only.
    From the docs, under "FILTER RULES":
   protect, 'P'
         specifies a pattern for protecting files from deletion. ...

(but I'm not very familiar with rsync)

What do you think? Both are not really backwards compatible, but option 2. atleast corrects the described behaviour.

@JamesIves
Copy link
Owner

JamesIves commented Apr 4, 2025

I like the name exclude, the clean-exclude option would need to linger for backwards compat reasons though, so both would need to work for the time being. Are you open to making a pull request?

@koenbollen
Copy link
Author

Sure, happy to give it a go.

@pratik-pdw
Copy link

Can I also exclude certain folders from being cleaned ?
For example if I have folders with name PR-1, PR-2 ? Can I exclude them before cleaning ?

@koenbollen
Copy link
Author

Yes, the exclude option also works for folders.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐝 This issue describes a bug. triage ⚠️
Projects
None yet
Development

No branches or pull requests

3 participants