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

PR: Update remote when syncing subrepos #20002

Merged
merged 1 commit into from
Nov 8, 2022

Conversation

mrclary
Copy link
Contributor

@mrclary mrclary commented Nov 7, 2022

When updating subrepos in Spyder using the current instructions (git subrepo clone), the remote specification in .gitrepo is not updated to reflect the cloned remote, but remains as the original upstream specification.

This can be an issue for any process that requires access to the remote repository at the commit and/or branch specified in .gitrepo, since that commit and/or branch may not exist on the (incorrectly) specified remote. The conda-based installers are an example of such a process.

This PR changes the instructions to use git subrepo pull ... -r <remote> -u to ensure that the remote specification is also updated.

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct: @mrclary

@dalthviz dalthviz added this to the v5.4.1 milestone Nov 7, 2022
@dalthviz dalthviz changed the title Update remote in .gitrepo of subrepos PR: Update remote in .gitrepo of subrepos Nov 7, 2022
@ccordoba12
Copy link
Member

A couple of comments:

  • What does -u do?
  • I tried to use gitsubrepo pull before instead of clone, but I think it doesn't work to update a subrepo here if your PR there was rebased (because the old parent commit is different after the rebase). Did you test that possibility?

@mrclary
Copy link
Contributor Author

mrclary commented Nov 8, 2022

  • What does -u do?
    -u, --update          Add the --branch and/or --remote overrides to .gitrepo
  • I tried to use gitsubrepo pull before instead of clone, but I think it doesn't work to update a subrepo here if your PR there was rebased (because the old parent commit is different after the rebase). Did you test that possibility?

Ahh, good point. I did not test that, but I'll be sure to do so.
We currently use the -f (force) flag, and it will persist with this PR. I'll check to see if that has any effect. Also, there is a -M (method) flag that can take values 'merge' or 'rebase'; 'merge' is the default. I will check to see whether this has any effect as well.

Ensure that .gitrepo remote is updated correctly
@mrclary
Copy link
Contributor Author

mrclary commented Nov 8, 2022

Okay, upon testing, I can confirm the following for git subrepo pull on a remote rebase:

  • Without -f (force), an error is raised because the previous commit no longer exists (no fast-forward merge is possible). This reproduces the condition that @ccordoba12 was concerned about.
  • Using -M rebase (method) has no effect.
  • With -f (force), the subrepo is successfully updated.

So, I think we should be good.

@CAM-Gerlach
Copy link
Member

As this PR seems focused on just updating the git-subrepo commands, and my experience with using git-subrepo pales next to @ccordoba12 and @dalthviz (whereas I do make much heavier use of git submodule), I'll leave the review in their much more capable hands

@ccordoba12
Copy link
Member

Okay, upon testing, I can confirm the following for git subrepo pull on a remote rebase ...
So, I think we should be good.

Great! Thanks for checking that @mrclary!

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mrclary!

@ccordoba12 ccordoba12 changed the title PR: Update remote in .gitrepo of subrepos PR: Update remote when syncing subrepos Nov 8, 2022
Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mrclary ! LGTM 👍

@dalthviz dalthviz merged commit 0eb5d12 into spyder-ide:5.x Nov 8, 2022
dalthviz added a commit that referenced this pull request Nov 8, 2022
@mrclary mrclary deleted the git-subrepo-fix branch November 8, 2022 20:32
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.

None yet

4 participants