fix(#551): fix issue with file deletion

[AndreasAugustin]

Description

Close #551

Fix issue with force deletion of files.

Checking #401 https://github.com/AndreasAugustin/actions-template-sync/pull/435/files the issue lies in some refactorings and race conditions. With the current implementation it checked the deleted files for an interval between the same hash what does not make any sense

Remark

For automation please see closing-issues-using-keywords

[aairey]

Was the happy and unhappy path tested ?

[AndreasAugustin]

Was the happy and unhappy path tested ?

Hi @aairey did you reveal any issue?

[aairey]

No, we are about to test ourselves but was wondering if testing already took place before the merge - as this is fixing a bug :).

[AndreasAugustin]

Hi @aairey did you run tests on your side? Did you reveal any issues?

[brajeev5]

Hi @AndreasAugustin

I tried using the delete feature of template sync with the following git remote pull parameters options, but I didn't achieve the expected result. Please find the respective logs for each of the git remote pull parameters options below.

git_remote_pull_params: --allow-unrelated-histories --strategy=recursive -X theirs

::warn::files to delete: .github/workflows/build-and-release.yml
deployment.config
.github/CODEOWNERS
.github/workflows/build-and-release.yml
.github/workflows/build-promotion.yml
.github/workflows/deployment.yml
.github/workflows/mend.yml
.github/workflows/snyk.yml
.releaserc.yaml
.github/workflows/README.md
.github/workflows/argo-deploy.yml
.github/workflows/docker-build.yml
.github/workflows/feature-deploy.yml
.github/workflows/maven-build.yml
.github/workflows/maven-publish.yml
.github/workflows/rundeck-deploy.yml
.github/workflows/sonarqube.yml
LICENSE
.github/workflows/maven-release.yml
rm: cannot remove '.github/workflows/build-and-release.yml': No such file or directory
rm: cannot remove '.github/workflows/README.md': No such file or directory
rm: cannot remove '.github/workflows/argo-deploy.yml': No such file or directory
rm: cannot remove '.github/workflows/docker-build.yml': No such file or directory
rm: cannot remove '.github/workflows/feature-deploy.yml': No such file or directory
rm: cannot remove '.github/workflows/maven-build.yml': No such file or directory
rm: cannot remove '.github/workflows/maven-publish.yml': No such file or directory
rm: cannot remove '.github/workflows/rundeck-deploy.yml': No such file or directory
rm: cannot remove '.github/workflows/sonarqube.yml': No such file or directory
rm: cannot remove '.github/workflows/maven-release.yml': No such file or directory

#It picks almost all the files from the destination repo instead on template sync repo. Not sure where it syncs from.

git_remote_pull_params: --allow-unrelated-histories --squash --strategy=recursive -X theirs

::warn::files to delete:
::endgroup::

#No files were detected.

git_remote_pull_params: --allow-unrelated-histories --strategy=recursive --no-edit

::warn::files to delete:
::endgroup::

#No files were detected.

git_remote_pull_params: --allow-unrelated-histories --squash --strategy=recursive -X theirs --no-edit --no-rebase

::warn::files to delete:

#No files were detected.

Kindly help me with the exact git remote pull params command to achieve the result.

[AndreasAugustin]

Hi @brajeev5 thanks for your comment please try git_remote_pull_params: --allow-unrelated-histories --strategy=recursive --no-edit (https://github.com/AndreasAugustin/actions-template-sync#force-deletion) cannot promise that it is working. This feature is somehow ugly and hard to implement.

Currently implemented without caching the last remote hash but I guess this need to be changed.
Seems the feature needs a complete redesign but I need lots of coffee. Not sure when this feature can be considered as stable.

[brajeev5]

Thank you @AndreasAugustin for your response. I have shared the results of git_remote_pull_params: --allow-unrelated-histories --strategy=recursive --no-edit in my previous comment, but that didn't work.

[aairey]

Maybe it is wrongfully comparing with HEAD here?

[AndreasAugustin]

Maybe it is wrongfully comparing with HEAD here?

theoretically no because the HEAD at this point in time is the hash from the remote (at least it should be and I tested that some time ago within https://github.com/AndreasAugustin/actions-template-sync/pull/435/files).

Algorithm idea
I think the I need to cache the last commit hash from last successful merge <last_merge_cash> from the source repository
Then it is possible (need to read if I remember right) to compare the current source tip with this hash and get all deleted files.
Then it is easy to delete them also on the target.

This will also fix possible issues with squashes and other history related changes on the target.

Currently I think an easy way doing that is using gh variable https://cli.github.com/manual/gh_variable
Not sure if race conditions are a big issue related that solution. Also need to check which permissions are needed https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/controlling-permissions-for-github_token#overview

Because of the need of a permission change this will possibly be a major release.

[aairey]

It is an approach and probably achieves some extra goals, yet it is adding some complexity and some other edge cases may arise.
But, I think the current goal is more minimal.

Let me re-iterate it for clarity:
We want to delete the files in the target repo that we git rm 'ed in the source repo.
Currently that is not coming across in the PRs that are being created in the target repo.

Just my 2 cents. The gh variable approach is still a potential valid one.

[AndreasAugustin]

It is an approach and probably achieves some extra goals, yet it is adding some complexity and some other edge cases may arise. But, I think the current goal is more minimal.

Let me re-iterate it for clarity: We want to delete the files in the target repo that we git rm 'ed in the source repo. Currently that is not coming across in the PRs that are being created in the target repo.

Just my 2 cents. The gh variable approach is still a potential valid one.

I totally understand and second that the idea is bringing some extra complexity. Nevertheless I do not find a easy solution which is working it seems. But if you have an idea. I totally looking forward to a PR from your side or just adding your ideas as comment 👍 😃