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

feat(sync-files): allow multiple destinations and speed up clone #295

Merged
merged 4 commits into from
Jun 7, 2024

Conversation

xmfcx
Copy link
Contributor

@xmfcx xmfcx commented Jun 7, 2024

Description

Speed up how?

Added --depth 1 to the git clone operation, performing a shallow clone, avoiding downloading entire history and branches of a repository.

It doesn't use anything more, so it should be safe.

Why iterate over multiple destinations? What was wrong with the existing approach?

Assume the following sync-files.yaml configuration:

- repository: autowarefoundation/autoware_common
  files:
    - source: .github/workflows/build-and-test.yaml
      dest: .github/workflows/build-and-test-daily.yaml
    - source: .github/workflows/build-and-test.yaml
      dest: .github/workflows/build-and-test-daily-arm64.yaml

It wants to copy from a source file into multiple destinations.

But when we run it with the existing configuration, it only generates the dest: .github/workflows/build-and-test-daily-arm64.yaml file. And completely ignores the first destination.

Proof

🖱️Click here to expand🔛

image

Input:

It only creates arm64 version:

Why does this occur?

To tackle this, I've ran https://gist.github.com/xmfcx/054a81b38ea583d39f792c6679c7bcde?permalink_comment_id=5081803#gistcomment-5081803 this script that I copied from the action itself. Put this in my autoware.universe directory.

Then put this https://gist.github.com/xmfcx/054a81b38ea583d39f792c6679c7bcde?permalink_comment_id=5081799#gistcomment-5081799 file in /tmp/sync-files.yaml

Also installed:

When I ran it, I saw:

source_path: .github/workflows/build-and-test.yaml
dest_path: .github/workflows/build-and-test-daily-arm64.yaml
/tmp/repository/.github/workflows/build-and-test.yaml and .github/workflows/build-and-test-daily-arm64.yaml are the same.
source_path: .github/workflows/build-and-test.yaml
dest_path: .github/workflows/build-and-test-daily-arm64.yaml
/tmp/repository/.github/workflows/build-and-test.yaml and .github/workflows/build-and-test-daily-arm64.yaml are the same.

Duplicate entries.

But then I ran it with the updated version from https://gist.github.com/xmfcx/054a81b38ea583d39f792c6679c7bcde?permalink_comment_id=5081803#file-sync-files-bash

And it was fixed:

source_path: .github/workflows/build-and-test.yaml
dest_path: .github/workflows/build-and-test-daily.yaml
/tmp/repository/.github/workflows/build-and-test.yaml and .github/workflows/build-and-test-daily.yaml are the same.
source_path: .github/workflows/build-and-test.yaml
dest_path: .github/workflows/build-and-test-daily-arm64.yaml
/tmp/repository/.github/workflows/build-and-test.yaml and .github/workflows/build-and-test-daily-arm64.yaml are the same.

We have both files generated.

Effects on system behavior

I can't see this breaking any existing behavior.

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

  • There are no open discussions or they are tracked via tickets.

After all checkboxes are checked, anyone who has write access can merge the PR.

@xmfcx
Copy link
Contributor Author

xmfcx commented Jun 7, 2024

To make better sense of things, I will share outputs of some commands so they are more concrete for everyone:

Files

🖱️Click here to expand🔛

The input file

mfc@mfc-leo:~$ cat /tmp/sync-files.yaml 
- files:
  - delete-orphaned: true
    dest: .github/workflows/build-and-test-daily.yaml
    post-commands: ''
    pre-commands: ''
    replace: true
    source: .github/workflows/build-and-test.yaml
  - delete-orphaned: true
    dest: .github/workflows/build-and-test-daily-arm64.yaml
    post-commands: ''
    pre-commands: ''
    replace: true
    source: .github/workflows/build-and-test.yaml
  - delete-orphaned: true
    dest: .github/workflows/check-build-depends.yaml
    post-commands: ''
    pre-commands: ''
    replace: true
    source: .github/workflows/check-build-depends.yaml
  - delete-orphaned: true
    dest: .github/workflows/clang-tidy-pr-comments.yaml
    post-commands: ''
    pre-commands: ''
    replace: true
    source: .github/workflows/clang-tidy-pr-comments.yaml
  - delete-orphaned: true
    dest: .github/workflows/clang-tidy-pr-comments-manually.yaml
    post-commands: ''
    pre-commands: ''
    replace: true
    source: .github/workflows/clang-tidy-pr-comments-manually.yaml
  - delete-orphaned: true
    dest: .github/workflows/update-codeowners-from-packages.yaml
    post-commands: ''
    pre-commands: ''
    replace: true
    source: .github/workflows/update-codeowners-from-packages.yaml
  - delete-orphaned: true
    dest: .pre-commit-config.yaml
    post-commands: ''
    pre-commands: ''
    replace: true
    source: .pre-commit-config.yaml
  - delete-orphaned: true
    dest: codecov.yaml
    post-commands: ''
    pre-commands: ''
    replace: true
    source: codecov.yaml
  ref: ''
  repository: https://github.com/autowarefoundation/autoware_common.git

Iterated last repository

mfc@mfc-leo:~$ cat /tmp/repo-config.yaml
files:
  - delete-orphaned: true
    dest: .github/workflows/build-and-test-daily.yaml
    post-commands: ''
    pre-commands: ''
    replace: true
    source: .github/workflows/build-and-test.yaml
  - delete-orphaned: true
    dest: .github/workflows/build-and-test-daily-arm64.yaml
    post-commands: ''
    pre-commands: ''
    replace: true
    source: .github/workflows/build-and-test.yaml
  - delete-orphaned: true
    dest: .github/workflows/check-build-depends.yaml
    post-commands: ''
    pre-commands: ''
    replace: true
    source: .github/workflows/check-build-depends.yaml
  - delete-orphaned: true
    dest: .github/workflows/clang-tidy-pr-comments.yaml
    post-commands: ''
    pre-commands: ''
    replace: true
    source: .github/workflows/clang-tidy-pr-comments.yaml
  - delete-orphaned: true
    dest: .github/workflows/clang-tidy-pr-comments-manually.yaml
    post-commands: ''
    pre-commands: ''
    replace: true
    source: .github/workflows/clang-tidy-pr-comments-manually.yaml
  - delete-orphaned: true
    dest: .github/workflows/update-codeowners-from-packages.yaml
    post-commands: ''
    pre-commands: ''
    replace: true
    source: .github/workflows/update-codeowners-from-packages.yaml
  - delete-orphaned: true
    dest: .pre-commit-config.yaml
    post-commands: ''
    pre-commands: ''
    replace: true
    source: .pre-commit-config.yaml
  - delete-orphaned: true
    dest: codecov.yaml
    post-commands: ''
    pre-commands: ''
    replace: true
    source: codecov.yaml
ref: ''
repository: https://github.com/autowarefoundation/autoware_common.git

Iterated last file item; (codecov.yaml) from above:

mfc@mfc-leo:~$ cat /tmp/file-config.yaml
delete-orphaned: true
dest: codecov.yaml
post-commands: ''
pre-commands: ''
replace: true
source: codecov.yaml

Some yq commands executed:

🖱️Click here to expand🔛
mfc@mfc-leo:~$ yq ".files[].dest" /tmp/repo-config.yaml
.github/workflows/build-and-test-daily.yaml
.github/workflows/build-and-test-daily-arm64.yaml
.github/workflows/check-build-depends.yaml
.github/workflows/clang-tidy-pr-comments.yaml
.github/workflows/clang-tidy-pr-comments-manually.yaml
.github/workflows/update-codeowners-from-packages.yaml
.pre-commit-config.yaml
codecov.yaml

mfc@mfc-leo:~$ export dest_file=.github/workflows/build-and-test-daily.yaml

mfc@mfc-leo:~$ yq ".files[] | select(.dest == \"$dest_file\")" /tmp/repo-config.yaml
delete-orphaned: true
dest: .github/workflows/build-and-test-daily.yaml
post-commands: ''
pre-commands: ''
replace: true
source: .github/workflows/build-and-test.yaml

@xmfcx xmfcx self-assigned this Jun 7, 2024
@xmfcx xmfcx marked this pull request as ready for review June 7, 2024 14:25
@xmfcx xmfcx requested a review from HansRobo June 7, 2024 14:27
Copy link
Member

@mitsudome-r mitsudome-r left a comment

Choose a reason for hiding this comment

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

LGTM

@xmfcx xmfcx merged commit dcd51f2 into main Jun 7, 2024
13 checks passed
@xmfcx xmfcx deleted the feat/improve-sync-files branch June 7, 2024 15:19
@xmfcx
Copy link
Contributor Author

xmfcx commented Jun 7, 2024

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