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

[BUG] all_old_new_renamed_files Gets all files renamed files from history #1030

Closed
3 tasks done
twcchu opened this issue Mar 16, 2023 · 14 comments · Fixed by #1040
Closed
3 tasks done

[BUG] all_old_new_renamed_files Gets all files renamed files from history #1030

twcchu opened this issue Mar 16, 2023 · 14 comments · Fixed by #1040
Labels
bug Something isn't working

Comments

@twcchu
Copy link

twcchu commented Mar 16, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Does this issue exist in the latest version?

  • I'm using the latest release

Describe the bug?

When getting all_old_new_renamed_files, a list of all previous renamed files is listed, even though the changes have been pushed to main.

To Reproduce

Beginning File Structure:

deploys
    - example.tfvars
    - es.tfvars
    - noceng.tfvars
  1. Change file name example.tfvars to example2.tfvars
  2. Run github action
- name: Save Changed tfvars Files
   id: files
   uses: tj-actions/changed-files@v35.7.1
   with:
     files: |
       deploys/*.tfvars
     include_all_old_new_renamed_files: 'true'

- name: Print renamed files
  run: |
  echo "all_old_new_renamed_files=${{ steps.files.outputs.all_old_new_renamed_files }}"
  echo "only_renamed_files=${{ steps.files.outputs.renamed_files }}"
  1. Check output
all_old_new_renamed_files=deploys/example.tfvars,deploys/example2.tfvars deploys/es1.tfvars,deploys/es.tfvars deploys/es.tfvars,deploys/es1.tfvars deploys/netops_observability.tfvars,deploys/noceng.tfvars

only_renamed_files=deploys/example2.tfvars
  1. Change file name back from example2.tfvars to example.tfvars and run Action
  2. Check output
all_old_new_renamed_files=deploys/example.tfvars,deploys/example2.tfvars deploys/example2.tfvars,deploys/example.tfvars deploys/es1.tfvars,deploys/es.tfvars deploys/es.tfvars,deploys/es1.tfvars deploys/netops_observability.tfvars,deploys/noceng.tfvars

only_renamed_files=

What OS are you seeing the problem on?

ubuntu-latest or ubuntu-20.04

Expected behavior?

Step 3:

all_old_new_renamed_files=deploys/example.tfvars,deploys/example2.tfvars

only_renamed_files=deploys/example2.tfvars

Step 5:

all_old_new_renamed_files=

only_renamed_files=

Relevant log output

changed-files
   Retrieving changes between ### (main) → ### (PLAT)

Anything else?

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@twcchu twcchu added the bug Something isn't working label Mar 16, 2023
@github-actions
Copy link
Contributor

Thanks for reporting this issue, don't forget to star this project if you haven't already to help us reach a wider audience.

@twcchu
Copy link
Author

twcchu commented Mar 16, 2023

May be similar to #972 but already using latest version.

@jackton1
Copy link
Member

jackton1 commented Mar 16, 2023

@twcchu Curious are you making the changes (rename) in a PR that hasn't been merged into the base branch?

Example

Branch (main) ← PR (my-feat) [Rename 1, Rename 2, ...]

@jackton1
Copy link
Member

jackton1 commented Mar 16, 2023

I assume you are trying to get the renamed file for the last change

i.e

  1. Change file name back from example2.tfvars to example.tfvars and run Action

Which would output

all_old_new_renamed_files=deploys/example.tfvars,deploys/example2.tfvars

To achieve this you can ideally compare the last remote commit to the current commit which included the rename from example2.tfvars back to example.tfvars

- name: Get Changed tfvars Files
   id: files
   uses: tj-actions/changed-files@v35.7.1
   with:
     files: deploys/*.tfvars
     include_all_old_new_renamed_files: true
     since_last_remote_commit: true  # NOTE: Without this setting, the diff is always performed between the target branch and the PR branch which is why you see the entire history of previously used names.  

Let me know if you run into any other issue

@jackton1 jackton1 closed this as not planned Won't fix, can't repro, duplicate, stale Mar 17, 2023
@twcchu
Copy link
Author

twcchu commented Mar 17, 2023

This branch was previously merged with main and is up to date with the main branch.
I added since_last_remote_commit: true and renamed the file to example2.tfvars again, but I still get

all_old_new_renamed_files=deploys/example.tfvars,deploys/example2.tfvars deploys/es1.tfvars,deploys/es.tfvars deploys/es.tfvars,deploys/es1.tfvars deploys/netops_observability.tfvars,deploys/noceng.tfvars

only_renamed_files=deploys/example2.tfvars

I am trying to get the renamed file compared to main so the since_last_remote_commit: true doesn't seem like what I am looking for.

@twcchu
Copy link
Author

twcchu commented Mar 17, 2023

I merged the previous PR into main and created a new PR renaming the file from example2.tfvars to example.tfvars.
This is the output:

all_old_new_renamed_files=deploys/example2.tfvars,deploys/example.tfvars, deploys/example.tfvars,deploys/example2.tfvars deploys/es1.tfvars,deploys/es.tfvars deploys/es.tfvars,deploys/es1.tfvars deploys/netops_observability.tfvars,deploys/noceng.tfvars

only_renamed_files=deploys/example.tfvars

It shows that deploys/example2.tfvars,deploys/example.tfvars was renamed in addition to all of the previous merges.

Relevant debug output:

##[debug]Set output target_branch = main
##[debug]Set output current_branch = PLAT
##[debug]Set output previous_sha = 5560070 ##latest commit on main branch
##[debug]Set output current_sha = 8e2a92b
...
/usr/bin/git diff --diff-filter=D --name-only 5560070...8e2a92b

@jackton1
Copy link
Member

jackton1 commented Mar 17, 2023

@twcchu Given that there are missing information to debug this I’ll recommend creating a public repository which replicates the issue with a minimal setup. I’ll reopen this issue and take a look once it’s setup. Thanks

@twcchu
Copy link
Author

twcchu commented Mar 17, 2023

I have created a repository to replicate the issue: twcchu/get-renamed-files#1

@twcchu
Copy link
Author

twcchu commented Mar 17, 2023

I changed the name of example.tfvars to example1.tfvars and then changed it back. I also changed es.tfvars to es1.tfvars.
The output is

all_old_new_renamed_files=example1.tfvars,example.tfvars es.tfvars,es1.tfvars example.tfvars,example1.tfvars
only_renamed_files=es1.tfvars

The expected output should be:

all_old_new_renamed_files= es.tfvars,es1.tfvars 
only_renamed_files=es1.tfvars

@jackton1
Copy link
Member

jackton1 commented Mar 18, 2023

@twcchu I believe the output is correct since the git diff is between the main (target) branch and the current commit which includes all renames that have happened even if it occurred to the same file. Is there a particular reason why you want to exclude the rename from the output ?

@twcchu
Copy link
Author

twcchu commented Mar 20, 2023

@jackton1 The output is including name changes from previous commits already merged into main. See second test PR: (twcchu/get-renamed-files#2).
I am expecting the output to only include name changes between the current commit and the main (target) branch like the output of only_renamed_files but including the old name of the renamed file. It is fine to include the changes to the same file in the current PR (although not ideal in our use case).

The reason we need both the old and new names of just the files updated in the current PR is because we are updating the names of Terraform Workspaces and we need both the old and new names. Including the names of previous PRs causes errors because the TF Workspaces are already renamed and can not be found.

@jackton1
Copy link
Member

jackton1 commented Mar 22, 2023

@twcchu I was able to replicate this, I'll work on pushing out the fix

@jackton1 jackton1 reopened this Mar 22, 2023
@jackton1 jackton1 linked a pull request Mar 22, 2023 that will close this issue
@jackton1
Copy link
Member

@twcchu This should now be available in the latest release

@twcchu
Copy link
Author

twcchu commented Mar 24, 2023

Great! Thank you!

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

Successfully merging a pull request may close this issue.

2 participants