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

only checkout necessary directories #908

Merged
merged 25 commits into from
Nov 29, 2024
Merged

Conversation

robbymilo
Copy link
Contributor

@robbymilo robbymilo commented Nov 28, 2024

for https://github.com/grafana/website/issues/10101

Looks as though https://github.com/grafana/grafana/blob/main/.dockerignore#L8 is making the deploy preview at grafana/grafana#97140 fail due to ignoring the dist dir.

As @jdbaldry suggested, we should only clone the necessary source dir. We could do this with sparse checkout, however root files are still included as the checkout step does not support the exclusions (see the comment). Instead we have a lovely find command that deletes anything expect the specified files.

Todo:

  • revert content change
  • revert ref change

Copy link
Contributor

github-actions bot commented Nov 28, 2024

@@ -66,6 +66,9 @@ jobs:

- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
if: github.event.action == 'opened' || github.event.action == 'synchronize'
with:
sparse-checkout-cone-mode: false # exclude root files
sparse-checkout: docs
Copy link
Member

Choose a reason for hiding this comment

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

We can make this configurable in the future if we like but every standard project and every one that I can think of (ignoring plugins) uses this structure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great, that means I can get rid of trimming the source_directory.

Comment on lines 84 to 93
# sparse checkout with cone mode disabled includes root files, even when using exclusions
# see https://github.com/actions/checkout/issues/1430#issuecomment-1756326892
- name: Keep only necessary files
if: github.event.action == 'opened' || github.event.action == 'synchronize'
shell: bash
run: |
find . -mindepth 1 -not \( -path "./${{ inputs.source_directory }}" -o -path "./${{ inputs.source_directory }}/*" -o -path "./deploy-preview-files" -o -path "./deploy-preview-files/*" \) -exec rm -rf {} +
ls -al
ls -al $trimmed_value

Copy link
Member

Choose a reason for hiding this comment

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

Can we also just consider trashing all files that aren't our docs directory?

Or perhaps another way is to do all the checking out into the default workspace but then copying what we want into a new subdirectory.

The workspace tree may then look like:

.
├── build
│   ├── deploy-preview-files
│   ├── dist
│   └── docs
│       └── sources
├── deploy-preview-files
└── grafana

Copy link
Member

Choose a reason for hiding this comment

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

In that previous comment, the grafana directory contains the grafana/grafana repo checkout (sparse or otherwise). The build directory is where we copy the deploy-preview-files; from the grafana/writers-toolkit checkout and docs/sources/ from the grafana/grafana checkout.

We then switch to that build directory before running all the rest of the action

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, that is what I'm trying with the find command. There is probably a cleaner way.

Copy link
Member

Choose a reason for hiding this comment

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

Oh right yeah, the find finds all files that aren't in our preferred list and removes them. That may be easiest actually because I recall that performing multiple steps in a subdirectory of an action is really verbose.

I think the find command along with maybe capturing the "to keep" directories in a Bash array for readability would be a good way to go.

I'm happy to pick this up tomorrow if you like. I came across this PR because I was hoping to give you a contribution for your review tomorrow but wanted to make sure that I wasn't stepping on your toes haha.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@robbymilo robbymilo force-pushed the robbymilo/deploy-preview branch from 6e1e5bb to de7c528 Compare November 29, 2024 08:24
Copy link
Member

@jdbaldry jdbaldry left a comment

Choose a reason for hiding this comment

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

Test output looks great to me!

@robbymilo robbymilo merged commit e97d9a9 into main Nov 29, 2024
5 checks passed
@robbymilo robbymilo deleted the robbymilo/deploy-preview branch November 29, 2024 08:47
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