Skip to content

Fix parent update #419

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

serbogus
Copy link

I had found that the parent pointer does not change after the pull. As a result, the next pull does re-sampling of old commits and ends up in merge conflicts in some cases. To fix the issue, i commented out the extra check in the update-gitrepo-file (# if [[ $upstream_head_commit == $output ]]; then).

This works in all my cases. It also works in all tests but "pull-merge.t". The test did sample the parent before pull. So, i modified it to sample the parent after the pull.

There was also an issue with 'push.t'. The push after pull had truncated history, because the parent pointer had been moved before the push is done. The solution for the issue was to add yet another parent pointer as 'push-parent'. So, the .gitrepo has another entry now.

On the way i had fixed a few issues:

  1. git 2.21 changed word casing in the 'Cc]ouldn't find remote ref' message. I fixed the regex

  2. branch... tests were failing sporadically at least with git 2.22. The order of transactions reported by rev-list in presence of branches was different from test to test. looks like --topo-order fixed it.

  3. error.t was failing inside a submodule, because there was a ref to '.dir'. So, the 'cd' command failed. I fixed the test.

As a result at the followup pull
   1. it resamples all the commits from the beginning of times in 'subrepo branch'
   2. the above causes merging issue.

The fix:
		remove an extra checking in the update-gitrepo-file while creating the parent

currentl failing the tsts:

test/branch-rev-list-one-path.t (Wstat: 0 Tests: 7 Failed: 1)
  Failed test:  7
test/error.t                   (Wstat: 0 Tests: 17 Failed: 1)
  Failed test:  17
  Parse errors: No plan found in TAP output
test/pull-merge.t              (Wstat: 0 Tests: 13 Failed: 1)
  Failed test:  8
test/push-after-init.t         (Wstat: 0 Tests: 0 Failed: 0)
  Parse errors: No plan found in TAP output
test/push-new-branch.t         (Wstat: 0 Tests: 0 Failed: 0)
  Parse errors: No plan found in TAP output
test/push.t                    (Wstat: 0 Tests: 17 Failed: 2)
  Failed tests:  3-4
…ed rev order in rev-list (at least in git 2.22).

the fix:
	add --topo-order qualifier to the rev-list to make the order to be stable across different test.

still failing tests:

test/error.t                   (Wstat: 0 Tests: 17 Failed: 1)
  Failed test:  17
  Parse errors: No plan found in TAP output
test/pull-merge.t              (Wstat: 0 Tests: 13 Failed: 1)
  Failed test:  8
test/push-after-init.t         (Wstat: 0 Tests: 0 Failed: 0)
  Parse errors: No plan found in TAP output
test/push-new-branch.t         (Wstat: 0 Tests: 0 Failed: 0)
  Parse errors: No plan found in TAP output
test/push.t                    (Wstat: 0 Tests: 17 Failed: 2)
  Failed tests:  3-4
The reason: there is no '.git' directory, just a .git ref.
fix:
	replace 'cd .dir' with 'cd $(git rev-parse --git-dir)'
…the subrepo pull.

I'd like this behavior to change. The parent should *change* after the pull.
	   test/push-after-init.t
	   test/push-new-branch.t
the reason:
	git 2.21 changed the message from Couldn't to couldn't

fix:
	fixed regex to catch both cases.
With the new scheme the pull will set up a new parent for the subrepo.
This causes all history before the parent to be truncated for the consequent push.

The way around it is to introduce yet another 'push-parent' in the .gitrepo file.
The push-parent is set up by the 'clone' and 'push' commands. The push uses it as subrepo_parent.

This way all tests passed.
@jakebailey
Copy link

I know this is an old PR (and appears to be inactive), but the parent check is affecting a subrepo I've got where the parent ref is never never updated until another re-clone (which is a shame). I'd love to see some version of this merged so I can depend on it.

@admorgan
Copy link
Collaborator

admorgan commented Apr 3, 2020

The reason the parent SHA isn't being updated is that you have changes that are on the local version that haven't been pushed upstream. Therefore the only safe thing to happen is for us to replay the changes and try to integrate the pull into them. I have found that I get a lot more merge failures when using the rebase method than the merge method. Unfortunately that particular patch is unlikely to ever be merged. The section I have marked for 4.2 is patch
1dd3c7d, the rest of them will be discarded.

@jakebailey
Copy link

jakebailey commented Apr 3, 2020

Your reply confuses me. I am pushing changes upstream (though not via subrepo push), and I'm using the merge method, not the rebase method.

I can't use subrepo push because:

  1. It modifies the .gitrepo file to change the remote path if --remote is provided. I split out changes from my subrepo into a PR from a fork to push upstream. When I do this, subrepo push creates a commit on the repo containing the subrepo changing the metadata. I do not want to push changes directly upstream without vetting.
  2. I want to squash the outgoing changes as the transferred commits are not clean at all. They include the git subrepo commits which I do not want to push upstream; subtree is an implementation detail of my repo, not the repo I've vendored.

Instead, I git subrepo branch, then squash in the worktree, then push the subtree branch to my fork of the external subrepo, where it gets pulled in. Then, the next time I subrepo pull, things should be getting synced, but only commit changes.

@jakebailey
Copy link

If you think my problem is out of scope for this PR or wouldn't be fixed by it, then I can open a new issue. I only commented here because I thought this PR was open, could be merged, and might have fixed my issue.

@admorgan
Copy link
Collaborator

admorgan commented Apr 3, 2020

This PR would work for your specific workflow, but it breaks a number of others. I would prefer you open another ticket so it would be easier to find the discussion. Unfortunately there isn't currently a great way to handle your issue and I think it is going to require some discussion.

@jakebailey
Copy link

jakebailey commented Apr 3, 2020

Sure, I'll open another issue with my usecase.

I did think up a workaround for now; run git subrepo branch and check if there are any differences between HEAD and subrepo.commit. If there are none, then it's safe to reclone (or maybe just change subrepo.parent directly).

EDIT: Not exactly as above, but it's close. Probably need to pull, see if there are still any differences, then reclone if safe (similarly to what I do now).

@thorsten-klein
Copy link

Hello @jakebailey,
+1
Do you have the exact commands you run? :-)

@jakebailey
Copy link

Sure, it's roughly this:

$ git subrepo clean <name>
# Split out the changes still applied to the subrepo but not upstreamed.
$ git subrepo branch <name>
$ pushd .git/tmp/subrepo/<name>
# Diff the worktree with upstream.
$ git diff --quiet <new hash of subrepo>
# If this succeeds, then:
$ popd
$ new_parent=$(git rev-parse HEAD)
$ git config --file path/to/.gitrepo subrepo.parent $new_parent
# Shift up the parent commit to simulate a forced reclone.
$ git commit -am "Update git-subrepo parent"

But, in a script. Alternatively if the previous commit was a git subrepo pull (what we do), the last step could --amend, or just roll back the last commit and reclone again as we know it's safe.

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.

4 participants