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

Change handling of submodules at upgrade #293

Merged
merged 1 commit into from
Jan 4, 2021

Conversation

erijo
Copy link
Collaborator

@erijo erijo commented Jan 4, 2021

What does this PR do?

Changes how submodules are handled during upgrade.

What issues does this PR fix or reference?

Previous Behavior

Upgrade would deinit submodules after moving the repo and then initialize them all. There are a couple of issues with this:

  • As -f is used during deinit, any local changes in the submodule would be lost.
  • The upgrade completes even if deinit fails (e.g. as seen in Upgrade to 3.0 broke yadm installation #285) and can leave the repo in an inconsistent state.
  • All submodules are initialized, even if they weren't initialized before the upgrade (minor issue).

New Behavior

  • The first step is now submodule absorbgitdirs. This makes sure that submodule deinit will not fail for modules that were cloned before being added.
  • Then submodule deinit is called for each submodule that's initialized. If an error happens the deinit is undone and upgrade fails.
  • After all submodules are deinit'ed, the upgrade continues as before.
  • Finally, submodules that were deinitialized in step 2 are re-initialized.

Have tests been written for this change?

Yes, see #284.

Have these commits been signed with GnuPG?

Yes


Please review yadm's Contributing Guide for best practices.

Start with doing "submodule absorbgitdirs" as otherwise "submodule
deinit" will fail if a module has been cloned first and later added as
a submodule (as it will then contain the .git dir instead of it being
under the superprojects .git dir).

Then try to deinit the submodules before moving the repo and abort the
upgrade if it fails for any submodule. Then do the move and finally
initialize the submodules that were initialized before the upgrade.

See yadm-dev#285
@TheLocehiliosan
Copy link
Member

@erijo - This looks great! I've added some documentation in 1c9dff7. Let me know if you think that is clear enough. This should be in the first version 3 bug fix release soon.

@erijo
Copy link
Collaborator Author

erijo commented Jan 4, 2021

Looks super!

@TheLocehiliosan TheLocehiliosan merged commit 0b4aa76 into yadm-dev:develop Jan 4, 2021
@erijo erijo deleted the submodule-upgrade branch January 4, 2021 23:17
TheLocehiliosan added a commit that referenced this pull request Jan 7, 2021
* Improve handling of submodules at upgrade (#284, #285, #293)
* Improve Zsh completions (#292, #298)
* Use stderr for error messages (#297)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants