744200 - hg pull shouldn't use -r $REV for tags

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[Rail Aliiev [:rail]]

I hit this a couple of time in staging, when I changed something in mozilla-release, tools, partner-repacks repos, retagged using -f, and wanted to rerun a build. hg pulls using -r $TAG and probably with help of hg share ends up pulling the old revision tagged using the same tag. The problem is visible when you use hgtool.py to checkout sources.

I think, removing revision=revision from all pull() calls should fix the problem. At least it helped me in staging.

Comment 1

[Rail Aliiev [:rail]]

Repro:

========================================================
cd /tmp
mkdir test
cd test
hg init
echo "v1" > 123.txt
hg add
hg commit -m "v1"
hg tag ver1
cd -
mkdir test-pull
cd test-pull
hg init
hg pull ../test
cd ../test
echo "v2" > 123.txt
hg commit -m "v2"
hg tag -f ver1
cd ../test-pull
hg pull -r ver1 ../test
hg up -C -r ver1
cat 123.txt # v1
cat ../test/123.txt # v2
========================================================

Patch incoming

Comment 2

[Rail Aliiev [:rail]]

Attachment: don't use revision for hg pull

remove revision=revision from pull() calls which converted to "hg pull -r" by cmd.extend(common_args(**kwargs))

Comment 3

[[e-mail] (:bhearsum)]

Comment on attachment 614018 [details] [diff] [review]
don't use revision for hg pull

Review of attachment 614018 [details] [diff] [review]:
-----------------------------------------------------------------

Can you add a test with the repro method in it, so we can make sure we don't break this again?

Comment 4

[Rail Aliiev [:rail]]

Comment on attachment 614018 [details] [diff] [review]
don't use revision for hg pull

I'll update the patch and add some tests

Comment 5

[Rail Aliiev [:rail]]

Attachment: don't pass revision

Comments incoming

Comment 6

[Rail Aliiev [:rail]]

Comment on attachment 614156 [details] [diff] [review]
don't pass revision

Review of attachment 614156 [details] [diff] [review]:
-----------------------------------------------------------------

::: lib/python/mozilla_buildtools/test/init_hgrepo.sh
@@ +13,5 @@
>  
>  echo "Hello world $RANDOM" > hello.txt
>  hg add hello.txt
>  hg commit -m "Adding hello"
> +hg tag TAG1

Add a tag somewhere in the middle

::: lib/python/mozilla_buildtools/test/test_util_hg.py
@@ +128,5 @@
>          rev = pull(self.repodir, self.wc, revision=self.revisions[0], update_dest=False)
>          self.assertEquals(rev, None)
>  
>          # We'll be missing the middle revision (on another branch)
> +        self.assertEquals(getRevisions(self.wc), self.revisions[:2] + self.revisions[2:])

hg tag creates an extra commit

::: lib/python/util/hg.py
@@ +226,5 @@
> +    pull_kwargs = kwargs.copy()
> +    if 'revision' in pull_kwargs:
> +        del pull_kwargs['revision']
> +
> +    cmd.extend(common_args(**pull_kwargs))

It turned out that we need to use revision here to update cwd if update_dest set to True

Comment 7

[Rail Aliiev [:rail]]

Probably this patch should be tested and maybe rewritten since we do want to pull by revision in try...

Comment 8

[Rail Aliiev [:rail]]

Attachment: Pull -r only for valid hg revisions

There is another approach:

* added another condition: remove -r from the cmd parameters only if revision doesn't represents a valid 40bit (shot, long) hex revision. Otherwise pull without -r. It should pull by revision in try and pull everything if named revisions are used.

* added more tests

Comment 9

[Chris AtLee [:catlee]]

Comment on attachment 615325 [details] [diff] [review]
Pull -r only for valid hg revisions

Review of attachment 615325 [details] [diff] [review]:
-----------------------------------------------------------------

::: lib/python/util/hg.py
@@ +231,5 @@
>      # Convert repo to an absolute path if it's a local repository
>      repo = _make_absolute(repo)
>      cmd = ['hg', 'pull']
> +    # Don't pass -r to "hg pull", except when it's a valid HG revision
> +    # pulling usin gtag names is dangerous

Can you expand on this? Why is pulling with -r dangerous with tag names?

Comment 10

[Rail Aliiev [:rail]]

Comment on attachment 615325 [details] [diff] [review]
Pull -r only for valid hg revisions

http://hg.mozilla.org/build/tools/rev/f1dd03a10485