code review 5731059: cmd/dist: use correct hg tag for go version

cmd/dist: use correct hg tag for go version

When looking for suitable tag always start
from current version, not the tip.

[brainman, 2012-03-05 00:46:19]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[brainman, 2012-03-05 00:47:24]

Here is issue report: http://goo.gl/fv9pB.

Alex

[minux1, 2012-03-05 02:26:17]

On Mon, Mar 5, 2012 at 8:47 AM, <alex.brainman@gmail.com> wrote:

> Here is issue report: http://goo.gl/fv9pB.
>
I've tested weekly.2012-02-14 with this CL, the output of 'go version' is:
go version weekly.2012-02-14

I think it fixes the problem, but it also removes the version hash from
output,
and we can't see if there is local modifications without that.

[brainman, 2012-03-05 03:34:09]

On 2012/03/05 02:26:17, minux wrote:
> 
> ..., but it also removes the version hash from output,
> and we can't see if there is local modifications without that.

This is different issue altogether. I agree, perhaps, that information is
useful, but it it not how it is documented in dist:

// If this tag matches the current checkout
// exactly (no "+" yet), don't show extra
// revision information.

So we suppose to show tag only, if current version has tag.

Also dist uses

hg log --template " +{node|short}" -r .

to fetch revision number, not

hg id

So, you will not get "+" at the end of it even if there are pending changes.
Lets wait for someone to decide what is correct approach here.

Alex

[minux1, 2012-03-05 03:48:26]

On Mon, Mar 5, 2012 at 11:34 AM, <alex.brainman@gmail.com> wrote:

> On 2012/03/05 02:26:17, minux wrote:
>
>  ..., but it also removes the version hash from output,
>>
>> and we can't see if there is local modifications without that.
>>
>
> This is different issue altogether. I agree, perhaps, that information
> is useful, but it it not how it is documented in dist:
>
> // If this tag matches the current checkout
> // exactly (no "+" yet), don't show extra
> // revision information.
>
> So we suppose to show tag only, if current version has tag.
>
That's correct, but regarding my test, I definitely have local
modifications (at least this CL
is applied), I think we should have a way to indicate that.
(Or we won't be able to tell that from people's issue reports.)

[brainman, 2012-03-05 03:57:06]

On 2012/03/05 03:48:26, minux wrote:
> >
> ..., I definitely have local
> modifications (at least this CL
> is applied), I think we should have a way to indicate that.
> (Or we won't be able to tell that from people's issue reports.)

I agree, but that is not what we do now. dist uses

hg log --template " +{node|short}" -r .

to fetch revision number, not

hg id

So, you will not get "+" at the end of it even if there are pending changes.
Lets decide what our version tag should look like first, then we could change
it.

Alex

[minux1, 2012-03-05 04:07:03]

On Mon, Mar 5, 2012 at 11:57 AM, <alex.brainman@gmail.com> wrote:
>
>  hg log --template " +{node|short}" -r .
>
> to fetch revision number, not
>
> hg id
>
> So, you will not get "+" at the end of it even if there are pending
> changes. Lets decide what our version tag should look like first, then
> we could change it.
>
I think the output of ’hg id -i‘ is perfect here, and look just like the
original output.
(Of course, I don't mind not displaying the version hash when it is at
precise the
weekly and no local modification is made)

[brainman, 2012-03-05 04:19:05]

On 2012/03/05 04:07:03, minux wrote:
> >
> I think the output of ’hg id -i‘ is perfect here, and look just like the
> original output.

Are you sure? We will have 2 "+" signs, like

weekly.2012-02-22 +5f45263f36dd+

Looks confusing to me.

Alex

[minux1, 2012-03-05 10:43:09]

On 2012/03/05 04:19:05, brainman wrote:
> On 2012/03/05 04:07:03, minux wrote:
> > >
> > I think the output of ’hg id -i‘ is perfect here, and look just like the
> > original output.
> 
> Are you sure? We will have 2 "+" signs, like
> 
> weekly.2012-02-22 +5f45263f36dd+
Sorry, my intent is:
weekly.2012-02-22 5f45263f36dd+
~~~~~~~~~~~~~~~~~
nearest tag/branch
                  ~~~~~~~~~~~~~
       if not exact on some tag, add this part.
       and if with local modifications, add '+'.
       this part is the output of 'hg id -i'

[rsc, 2012-03-05 21:17:22]

On Sun, Mar 4, 2012 at 19:46,  <alex.brainman@gmail.com> wrote:
> cmd/dist: use correct hg tag for go version
>
> When looking for suitable tag always start
> from current version, not the tip.

What version string did you see that prompted this?

You wrote that you are trying to get rid of the second plus
in weekly.2012-02-22 +5f45263f36dd+, but I don't think the
current code would generate that.  It would find tag = "weekly.2012-02-22"
in the code you are editing, and then after that it would execute

hg log --template '+{node|short}' -r .

which does not add a trailing +.  My tree always has local
modifications but my current Go version string is

go version weekly.2012-03-04 +faadc270f4d8

(with no trailing +).

Russ

[brainman, 2012-03-05 22:20:12]

On 2012/03/05 21:17:22, rsc wrote:
> 
> What version string did you see that prompted this?
> 

Here is issue report: http://goo.gl/fv9pB.

hg clone -u weekly.2012-02-14 http://code.google.com/p/go
cd go/src
./make.bash
../bin/go version

the output is still: 
go version weekly.2012-02-22 +43cf9b39b647

(it meant to be "go version weekly.2012-02-14 ...")

The problem here is that our current code works, if we are on the tip. But
breaks, if we go to the past.

> You wrote that you are trying to get rid of the second plus
> in weekly.2012-02-22 +5f45263f36dd+, but I don't think the
> current code would generate that. ...

"+" at the end of revision is something else completely and is not part of this
CL (yet). minux.ma suggested (while commenting on this CL) that we should change
our logic to have end "+". It is not how dist is at this moment, and I am not
sure, it is desirable.

> ...  My tree always has local
> modifications but my current Go version string is
> 
> go version weekly.2012-03-04 +faadc270f4d8
> 
> (with no trailing +).
> 

I take it, you do not want trailing +. That is OK. What about this CL as is?

Alex

[rsc, 2012-03-05 22:48:01]

LGTM

I do not want the trailing +
(I worked hard to get rid of it),
so this CL looks good as is.

Thanks for the clarification.

[brainman, 2012-03-06 06:21:48]

*** Submitted as http://code.google.com/p/go/source/detail?r=f46217216512 ***

cmd/dist: use correct hg tag for go version

When looking for suitable tag always start
from current version, not the tip.

R=minux.ma, rsc
CC=golang-dev
http://codereview.appspot.com/5731059

[minux1, 2012-03-06 10:49:25]

On Tue, Mar 6, 2012 at 6:20 AM, <alex.brainman@gmail.com> wrote:
>
> The problem here is that our current code works, if we are on the tip.
> But breaks, if we go to the past.
>
>  You wrote that you are trying to get rid of the second plus
>> in weekly.2012-02-22 +5f45263f36dd+, but I don't think the
>> current code would generate that. ...
>>
>
> "+" at the end of revision is something else completely and is not part
> of this CL (yet). minux.ma suggested (while commenting on this CL) that
> we should change our logic to have end "+". It is not how dist is at
> this moment, and I am not sure, it is desirable.
>
>  ...  My tree always has local
>>
>> modifications but my current Go version string is
>>
>
>  go version weekly.2012-03-04 +faadc270f4d8
>>
>
>  (with no trailing +).
>>
>
> I take it, you do not want trailing +. That is OK. What about this CL as
> is?
>
Sorry, but my real intent is not where the '+' should go, but it should
have '+'
if the repo has local modifications. I merely suggested that we use output
of 'hg id -i' to serve that purpose, which unfortunately has a trailing '+'
that
you do not like.

I updated the repo to weekly.2012-02-14, applied this CL, build it, and the
output of 'go version' is:
go version weekly.2012-02-14

I think it really should have a indication that I've made local
modifications.

[rsc, 2012-03-06 14:24:47]

On Tue, Mar 6, 2012 at 05:49, minux <minux.ma@gmail.com> wrote:
> I think it really should have a indication that I've made local
> modifications.

Sorry, but no.  We did that for a while.
It gets annoying that the first time you rebuild
the compilers after you make a change, all the
binaries you just built are stale.

Also sometimes the editing that goes on during
the bootstrap phase can make hg think the tree is
modified when it's really not.

Russ