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

support force-cleaning git repos #632

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

bugfood
Copy link

@bugfood bugfood commented Dec 29, 2023

Summary

Add support to vcsrepo's git provider to enforce the status of the git repo. The supported statuses are "default_clean" (to clean the repo, while ignoring files in .gitignore and "ignore" (to do nothing).

Additional Context

This change is written to support other cleaning methods in the future, if desired.

The first commits fix up the existing documentation and unit tests a bit, then the last commit adds the new feature.

Checklist

  • 🟢 Spec tests.
  • 🟢 Acceptance tests.
  • Manually verified. (For example puppet apply)

These were nearly sorted to start with.
Before this change, tests for 'ensure' values other than 'present' are
nested under 'when with an ensure of present', resulting in:

Puppet::Type::Vcsrepo::ProviderGit
  when with an ensure of present
    when with an ensure of present - with a revision that is a remote branch
      executes 'git clone' and 'git checkout -b'
[...]
    when with an ensure of bare - with revision
      raises an error
    when with an ensure of bare - without revision
      justs execute 'git clone --bare'
    when with an ensure of bare - without a source
      executes 'git init --bare'
    when with an ensure of mirror - with revision
      raises an error
    when with an ensure of mirror - without revision
      justs execute 'git clone --mirror'
    when with an ensure of mirror - without a source
      raises an exeption
    when with an ensure of mirror - with multiple remotes
      executes 'git clone --mirror' and set all remotes to mirror
Neither of these tests set `ensure` to any value.
@bugfood bugfood requested review from bastelfreak, smortex and a team as code owners December 29, 2023 21:25
@bugfood bugfood force-pushed the force-clean branch 3 times, most recently from 98ef182 to c46c9eb Compare December 29, 2023 22:07
@bugfood
Copy link
Author

bugfood commented Dec 29, 2023

There are a couple things I couldn't do quite as well as I would like.

  1. I don't know the best way to document the possible values of a property. I don't think that newvalue accepts a desc attribute, but I could be wrong. I asked about this in the puppet community slack:
    https://puppetcommunity.slack.com/archives/C0W298S9G/p1703790588151839

  2. This new feature has an interaction with the revision property. If both revision and repository_status are applied, then the exact behavior depends on the order in which the properties are applied:

  • If revision happens first, then the repo is cleaned according to the .gitignore of the target revision.
  • If repository_status happens first, then the repo is cleaned according to the .gitignore of the original revision.
    In either case, revision does a git checkout --force by default, so some local changes may be clobbered anyway.
    Experimentally, revision is currently applied first, due to being listed first in the file, but I don't think there's any guarantee of this behavior. I asked about this in the puppet community slack:
    https://puppetcommunity.slack.com/archives/C0W298S9G/p1703791951239809

I don't think either of these issues should gate merging the PR, but I'm definitely willing to try any suggestions.

Thank you,
Corey

@bugfood
Copy link
Author

bugfood commented Dec 29, 2023

The remaining acceptance test failures seem to be unrelated to this PR.

-Corey

@bugfood
Copy link
Author

bugfood commented Jan 2, 2024

I updated the PR to support cleaning submodules as well.

@bugfood
Copy link
Author

bugfood commented Jan 11, 2024

Changes in new update:

  • additionally, use git reset --hard HEAD to remove local changes to tracked files
  • update and fix unit test
  • reword description in README file

-Corey

This resets changes to both tracked and un-tracked files. Submodules are
cleaned as well, when enabled.

For untracked files, 'git clean -fd' should have the most reasonably
expected result (files in .gitignore are still ignored). This change is
written to support other cleaning methods in the future, if desired.
@bugfood
Copy link
Author

bugfood commented Jan 11, 2024

Changes in new update:

  • update submodules to match the respective revisions specified in the containing repo
  • only check/clean submodules when submodules are enabled

-Corey

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants