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

Inconsistent results on multiple target repos #103

Closed
bytestream opened this issue Jun 1, 2021 · 4 comments · Fixed by #237
Closed

Inconsistent results on multiple target repos #103

bytestream opened this issue Jun 1, 2021 · 4 comments · Fixed by #237
Labels
bug Something isn't working released

Comments

@bytestream
Copy link

Hi,

I've integrated this action and on the first run it's produced some inconsistent results.

You can see the workflow file here: https://github.com/supportpal/language-files/blob/master/.github/workflows/sync.yml

The initial workflow run logs: https://github.com/supportpal/language-files/runs/2704553466?check_suite_focus=true

You can see that some branches were updated, and others were not. None of the branches contained a .github/workflows/test.yml file at the time the workflow ran. I expected it to be copied from master into each specified branch.

Here's the log comparing two branches:

supportpal/language-files:nl: EXEC: "GIT_LFS_SKIP_SMUDGE=1 git clone --depth 1  -b nl https://***@github.com/supportpal/language-files.git tmp-1622377767181/supportpal/language-files-nl" IN "./"
supportpal/language-files:nl: OUTPUT: "null"
supportpal/language-files:nl: FILE_PATTERNS [
  "/^.github\\/workflows\\/test.yml$/"
]
supportpal/language-files:nl: MATCHING FILES: []
supportpal/language-files:nl: REMOVE FILES []
supportpal/language-files:nl: EXEC: "git status --porcelain" IN "tmp-1622377767181/supportpal/language-files-nl"
supportpal/language-files:nl: copy tmp-1622377767181/supportpal/language-files/.github/workflows/test.yml to tmp-1622377767181/supportpal/language-files-nl/.github/workflows/test.yml
supportpal/language-files:nl: OUTPUT: "null"
supportpal/language-files:nl: NO CHANGES DETECTED
supportpal/language-files:de: EXEC: "GIT_LFS_SKIP_SMUDGE=1 git clone --depth 1  -b de https://***@github.com/supportpal/language-files.git tmp-1622377767181/supportpal/language-files-de" IN "./"
supportpal/language-files:de: OUTPUT: "null"
supportpal/language-files:de: FILE_PATTERNS [
  "/^.github\\/workflows\\/test.yml$/"
]
supportpal/language-files:de: MATCHING FILES: []
supportpal/language-files:de: REMOVE FILES []
supportpal/language-files:de: EXEC: "git status --porcelain" IN "tmp-1622377767181/supportpal/language-files-de"
supportpal/language-files:de: copy tmp-1622377767181/supportpal/language-files/.github/workflows/test.yml to tmp-1622377767181/supportpal/language-files-de/.github/workflows/test.yml
supportpal/language-files:de: OUTPUT: "null?? .github/
"
supportpal/language-files:de: CHANGES DETECTED
supportpal/language-files:de: COMMIT CHANGES...
supportpal/language-files:de: EXEC: "git config --local user.name "bytestream" && git config --local user.email "bytestream@users.noreply.github.com" && git add -A && git status && git commit --message "Update file(s) from "supportpal/language-files"" && git push" IN "tmp-1622377767181/supportpal/language-files-de"
supportpal/language-files:de: OUTPUT: "nullOn branch de
Your branch is up to date with 'origin/de'.

Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
	new file:   .github/workflows/test.yml

[de d8aa44a] Update file(s) from supportpal/language-files
 1 file changed, 36 insertions(+)
 create mode 100644 .github/workflows/test.yml
"
supportpal/language-files:de: CHANGES COMMITED

Any ideas what happened here?

@adrianjost
Copy link
Owner

I have noticed a couple of thinks on a first quick investigation. I haven't developed this project for a while, so I can't immediately tell what the real cause is, but here are my thoughts:

  • all the target repos are identical and just the branch differs. I am not sure if I ever tested this case, because I didn't had the use-case myself yet but see it as absolutely valid. I can imagine, that I am cloning each repo only once, so that there might be some conflicts when using the same repo multiple times with different branches. But this is just an idea that needs to be validated. If it is the case, the fix should be simple
  • I had some reliability issues myself from time to time but couldn't really track down the reason. My latest thought that comes to mind is, that the parsing of the git cmd results wasn't reliable, because it depended on the raw console output. At some point, I should switch to a package specifically made to execute git commands by script.

I will try to take a closer look within the next week, because this is a use-case that I think is important for most people and should work reliably.

If you find out anything else, or can support me by digging into the source code yourself I would really appreciate it, since this is a pure free time project for me.

And if it helps you: Here is the sync-action I am using myself regularly so sync files across my repositories: https://github.com/adrianjost/.github/blob/master/.github/workflows/sync.yml

@bytestream
Copy link
Author

Thanks for taking the time to reply.

It must be something to do with copy files. Git status returns an empty response if there are no changes which is what happened in the first two cases.

My observations are:

  • there could be a missing async / await, but this seems unlikely from a look over the code and the log message order is the same in all cases
  • git status returned an empty response which means there are no changes. This suggests to me that either parsing git status is unreliable, or most likely... the result of copying files is not checked. It's possible that there was some IO issue which prevented the copy operation so git status correctly returned no results
  • the issue only occurred on the first two branches

If I had to work on one hypothesis it would be ensuring that the copy files operation is successful. See

await fs.mkdir(path.dirname(to), { recursive: true });

Given the lack of a reliable test case I think this will be difficult to track down so I wouldn't spend too much time on it. I would just try to improve the copy files checks and then close this issue without further thought.

@adrianjost adrianjost added the bug Something isn't working label Mar 19, 2023
@immjs
Copy link
Contributor

immjs commented Jul 30, 2023

I found the issue! As you can see in this extract of the logfiles,

supportpal/language-files:nl: EXEC: "git status --porcelain" IN "tmp-1622377767181/supportpal/language-files-nl"
supportpal/language-files:nl: copy tmp-1622377767181/supportpal/language-files/.github/workflows/test.yml to tmp-1622377767181/supportpal/language-files-nl/.github/workflows/test.yml
supportpal/language-files:nl: OUTPUT: "null"

your Git Status is running before copying the file. And then as you can see in this bit of code:
https://github.com/adrianjost/files-sync-action/blob/84a36626307cfb3da1eee0ff9d60b72c9eba1456/src/index.js#L41C1-L53C8
it's because your copy promise is actually not awaited: you are effectively doing:

await Promise.all([Promise, Promise[]])

but Promise[] is not something that is await-able and therefore it does not await it.

I opened PR #237 to fix this

adrianjost pushed a commit that referenced this issue Jul 31, 2023
@github-actions
Copy link

🎉 This issue has been resolved in version 2.0.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants