Add option to ignore changes to the vendor directory

[dkotter]

Description of the Change

Multiple times when using this Action we've run into issues where trying to push out changes to the readme.txt file don't work due to Other files have been modified; changes not deployed. Finally looked into this closer and the problem is the same as described in #49.

If you have a plugin that uses composer to install non-dev dependencies (or just use it for the autoloader), when running this Action, you'll typically run composer install --no-dev as part of a build step. Composer will use a hash for some of the function names and this hash may change each time things are built.

When this Action then compares the files from the svn repo to what is in Github, those composer generated files don't match up due to these different hash values, resulting in the above error.

You can get around this by utilizing the existing IGNORE_OTHER_FILES argument but the downside here, at least in the way we have things set up, anytime we push out a new release the changes to the readme.txt file will get deployed first by this Action and then the actual release will go out later. Not sure that will cause problems but it's not ideal. I did experiment with conditionally setting the value of IGNORE_OTHER_FILES when needed and while this did work, it's not the ideal workflow.

What I eventually decided on was introducing a new configuration option (IGNORE_VENDOR_DIR) that defaults to false (and thus won't change any existing behavior). But if set to true, after we've synced files from Github to our checked out svn repo but before checking to see if there's other file changes we don't want, we revert any changes made to the vendor directory.

Closes #49

How to test the Change

Not sure if there's a best practice to how to test things here without accidentally deploying things to the .org svn repo but here's the steps I took that others can follow:

1. Checkout this branch
2. In the deploy.sh file, comment out lines 11-14 and 16-19. These lines deal with setting the svn username and password, which we don't want to do to avoid accidentally committing changes
3. In the deploy.sh file, comment out lines 188-191. These lines are what actually commit the changes. They shouldn't work without a proper username and password but I feel better removing those
4. Clone a plugin that uses composer and run whatever production build steps this plugin needs. Goal is to end up with a plugin built the same way you would before deploying it to .org
5. We then need some environment variables set:

• SLUG: set this to a valid plugin slug that you have cloned locally; ex export SLUG='ad-refresh-control'
• GITHUB_WORKSPACE: set this to the path locally to the cloned plugin; ex export GITHUB_WORKSPACE='/Users/user/sites/oss/app/public/wp-content/plugins/Ad-Refresh-Control'

6. Then run the script: ./deploy.sh. You should get an error about other files being modified and right above that line, you should see some vendor directories being flagged
7. Utilize the new configuration option from this PR: export IGNORE_VENDOR_DIR=true
8. Run ./deploy.sh again. You should get a message saying Nothing to deploy! and right above that you should see lines about reverting the vendor directory

Changelog Entry

Added - New configuration option, IGNORE_VENDOR_DIR, that can be used to ignore any changes to the vendor directory.

Credits

Props @dkotter, @cadic

Checklist:

• I agree to follow this project's Code of Conduct.
• I have updated the documentation accordingly.
• I have added tests to cover my change.
• All new and existing tests pass.

[ravinderk]

@dkotter The purpose of this GitHub action is to update WordPress assets, right? If so, why can't we silently ignore vendor changes?

[cadic]

@ravinderk Hmm, I suppose, the action should stop if any of composer dependencies in /vendor changed. Which will also reflect in the composer.json and .lock files. In this case I agree, the action could ignore /vendor entirely and rely just on changes in composer config files.

[dkotter]

The purpose of this GitHub action is to update WordPress assets, right?

Yes, the purpose of this Action is two things:

1. Update WordPress.org assets (this is different from any assets a plugin may use)
2. Update readme.txt files (useful for things like WordPress tested bumps)

This Action tries to be smart about these changes and it won't deploy if other file changes are found. This is to avoid issues where you deploy things before they are ready.

As an example, the workflow we normally follow is we branch from develop and merge PRs back into develop as they are ready. Once a release is ready, we merge version bumps and changelog updates into develop and then merge develop in to trunk.

We have this Action in place that fires on any merge into trunk. We then have another Action in place that fires anytime a release is done on GitHub and pushes that release to .org. If this Action didn't ignore other file changes, what would happen is the readme.txt file would be deployed after the merge into trunk but the full release wouldn't happen until after the release is done on GitHub. This means the readme.txt file goes out sooner than it should, containing version bumps and changelog updates.

If so, why can't we silently ignore vendor changes?

The issue with forcing this is a scenario where someone wants to push out a new release and the only thing they changed was composer dependencies. As an example, say you use a 3rd party SDK that you manage with composer and you want to push out a new version of that. When those changes are merged into trunk, this would trigger this Action and the readme.txt file would be deployed. But the actual composer changes wouldn't be deployed until the release is made.

This isn't an ideal scenario, especially for users that merge into trunk but maybe take multiple hours/days to actual cut a release, the readme.txt file changes would be live sooner than they should be.

By introducing a new config option here, people have more control over what gets ignored.

That said, another option I debated on and may be a better approach is providing a new config option that allows you to specify specific files or directories that should be ignored. This would allow more flexibility beyond just the vendor directory

[ravinderk]

@dkotter, I apologize for any confusion in my previous comment. I realize now that I wasn't specific about which files to ignore in the vendor directory. I intend to ignore the dynamic files generated by the composer.

The list of files and directories to ignore includes:

• vendor/autoload.php
• vendor/composer

@cadic also highlighted that these files are causing issues.

The proposed solution functions effectively without needing a new config parameter since the process automatically halts upon detecting any changes in the library within the vendor directory.

Please let me know if there are any use cases I might be overlooking.

[dkotter]

I think this all sounds fine, tried to think through all scenarios to see if there's a situation where this may cause problems but couldn't come up with any.

If you'd like to work on a PR to make those changes, that would be great, and then we can probably just close this one out.

[ravinderk]

I think this all sounds fine, tried to think through all scenarios to see if there's a situation where this may cause problems but couldn't come up with any.

If you'd like to work on a PR to make those changes, that would be great, and then we can probably just close this one out.

@dkotter, I will plan to open a new pull request to address this issue and subsequently close it.

cc: @jeffpaul

[ravinderk]

@dkotter I created a pull request as discussed.