code review 4969065: hgpatch: workaround for 'hg -q incoming' on windows.

hgpatch: workaround for 'hg -q incoming' on windows.
Mercurial provides binary package and source package for windows.
If using binary package, hg.exe return 1 for exit code correctly.
But hg.bat does not return 1.
This is workaround to get exit code from batch file like following.

        cmd /v:on /c hg -q incoming & exit !ERRORLEVEL!

This is working fine for both packages.

[mattn, 2011-09-08 02:20:16]

Hello golang-dev@googlecode.com, rsc@golang.org (cc:
golang-dev@googlegroups.com),

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

[rsc, 2011-09-08 02:24:17]

Do most people have hg.exe or hg.bat?
Is the buggy hg behavior common enough
to warrant a special case here?

Russ

[mattn, 2011-09-08 02:36:06]

This is behavior of windows.

If the user install hg from source code, 'setup.py' make 'hg.bat' script to 
'c:\python27\scripts\hg.bat'.
And if the user install hg from binary package, 'hg.exe' is installed to 
'c:\program files\mercurial\hg.exe'.
So most people have either of them.

GetExitCodeProcess() can't exit code from batch file.
To get it, hg.bat should call following at last line.

    exit /b 1

    or

    exit /b 0

I think, This is not work a bug of mercurial, And this is not a bug of 
'script.py'.

[brainman, 2011-09-08 02:58:22]

On 2011/09/08 02:36:06, mattn wrote:
> 
> GetExitCodeProcess() can't exit code from batch file.
> To get it, hg.bat should call following at last line.
> 
>     exit /b 1
> 
>     or
> 
>     exit /b 0
> 

If someone creates hg.bat, that always return exitcode=0, I am happy for hgpatch
to report success all the time. That is what they are asking for, aren't they?

Alex

[mattn, 2011-09-08 03:15:44]

On Thursday, September 8, 2011 11:58:22 AM UTC+9, brainman wrote:
>
> If someone creates hg.bat, that always return exitcode=0, I am happy for
> hgpatch to report success all the time. That is what they are asking
> for, aren't they?
>
No,  As long as that hg is batch file, GetExitCodeProcess() can't get exit 
code.
That's not the point. :)


 

[brainman, 2011-09-08 03:25:35]

On 2011/09/08 03:15:44, mattn wrote:
> 
> No,  As long as that hg is batch file, GetExitCodeProcess() can't get exit 
> code.

I know nothing about batch programming, but didn't you say that batch should do

exit /b 1

?

> That's not the point. :)

What is the point?

Alex

[rsc, 2011-09-08 03:32:39]

This only matters for people developing
Go on Windows, since hgpatch is only used
by and only exists for the codereview extension.

Can we tell people developing on Windows
that hg.bat is buggy and they should use hg.exe?
There are certainly plenty of other hg bugs we
tell people to work around (usually by upgrading
to a different hg).

Russ

[mattn, 2011-09-08 03:34:12]

Are you saying mercurial MUST provide hg.bat using "exit /b" ?
Then, that's right :)

However, golang have a lot of problems for windows user.
If user want to use hg.exe, they must do:

    https://groups.google.com/d/msg/golang-dev/rr-7N-UJk-E/paNKZQD1h68J

And If user want to use hg installed from source code, we must wait next 
release of mercurial.

On Thursday, September 8, 2011 12:25:35 PM UTC+9, brainman wrote:
>
> On 2011/09/08 03:15:44, mattn wrote:
>
> > No,  As long as that hg is batch file, GetExitCodeProcess() can't get
> exit
> > code.
>
> I know nothing about batch programming, but didn't you say that batch
> should do
>
> exit /b 1
>
> ?
>
> > That's not the point. :)
>
> What is the point?
>
> Alex
>
> http://codereview.appspot.com/4969065/
>
>

[brainman, 2011-09-08 03:42:34]

On 2011/09/08 03:34:12, mattn wrote:
> Are you saying mercurial MUST provide hg.bat using "exit /b" ?

All I am saying is, if program (batch or otherwise) wants to report an error, it
must do so in a conventional way = "exit code". If program always returns
exit_code=0, then it, obviously, do not care about informing its callers about
results of execution.

> 
> However, golang have a lot of problems for windows user.
> If user want to use hg.exe, they must do:
> 
>     https://groups.google.com/d/msg/golang-dev/rr-7N-UJk-E/paNKZQD1h68J
> 
> And If user want to use hg installed from source code, we must wait next 
> release of mercurial.
> 

I have no argument here. Please, fix it, if you know how.

Alex

[mattn, 2011-09-08 03:55:58]

On Thursday, September 8, 2011 12:42:35 PM UTC+9, brainman wrote:
>
> On 2011/09/08 03:34:12, mattn wrote:
> > Are you saying mercurial MUST provide hg.bat using "exit /b" ?
>
> I have no argument here. Please, fix it, if you know how.
>

Ok, I'll report this problem to python dev-team, but the patch will be 
applied to setuputils or distutils in python module if they accept it.
And I guess, they will say 'Get from %ERRORLEVEL% !'.
Perhaps, it will take many times until that windows user can use hgpatch 
correctly.

[brainman, 2011-09-08 04:12:21]

On 2011/09/08 03:55:58, mattn wrote:
> 
> ... it will take many times until that windows user can use hgpatch 
> correctly.
> 

I do not use codereview on windows. So, perhaps, I am not the right person to
say NO. But I always try to fix the cause, not the consequences.

Alex

[mattn, 2011-09-08 04:27:10]

Sorry, I just noticed important thing.

I added following line into hg.bat,

   exit /b 1

But hgpatch failed...

Hmm


On Thursday, September 8, 2011 1:12:21 PM UTC+9, brainman wrote:
>
> On 2011/09/08 03:55:58, mattn wrote:
>
> > ... it will take many times until that windows user can use hgpatch
> > correctly.
>
>
> I do not use codereview on windows. So, perhaps, I am not the right
> person to say NO. But I always try to fix the cause, not the
> consequences.
>
> Alex
>
> http://codereview.appspot.com/4969065/
>
>

[mattn, 2011-09-08 05:01:40]

Hmm,

On the second thought, GetExitCodeProcess() can't get exit code from batch 
file.
seems impossible if batch file do "exit /b 1"

Try following.
--- foo.c ---
#include <string.h>
#include <stdlib.h>
int main(int argc, char* argv[]) { exit(atoi(argv[1])); }
-------------

--- bar.bat ---
@echo off
exit /b %1
---------------

--- baz.bat ---
@echo off
exit %1
---------------

--- test.go ---
package main

import "exec"

func run(command string, args... string) bool {
_, err := exec.Command(command, args...).CombinedOutput()
return err == nil
}

func main() {
println(run(`.\foo.exe`, "0"))
println(run(`.\foo.exe`, "1"))
println(run(`.\bar.bat`, "0"))
println(run(`.\bar.bat`, "1"))
println(run(`.\baz.bat`, "0"))
println(run(`.\baz.bat`, "1"))
}
---------------

Then, test program output following.

true
false
true
true
true
false

This can get with "exit 1", But it close cmd.exe if run on command prompt.
Thus, we should get exit code from batch file, and pass it to "batch file 
interpreter(meaning cmd.exe).

[rsc, 2011-09-08 18:02:39]

It looks to me like hg.bat should say exit, and not exit /b.
exit /b looks like it is only really meant to be used from
within batch files invoked by other batch files.

Russ

[mattn, 2011-09-08 18:14:18]

Yes, If pass exit code to cmd.exe, we should call as:

cmd /c call hg.bat ...

But this is talking about batch files.

My workaround above is working on both.

[peterGo, 2011-09-09 13:28:33]

Russ,

On 2011/09/08 03:32:39, rsc wrote:
> Can we tell people developing on Windows
> that hg.bat is buggy and they should use hg.exe?

SGTM

Peter

On 2011/09/08 03:32:39, rsc wrote:
> This only matters for people developing
> Go on Windows, since hgpatch is only used
> by and only exists for the codereview extension.
> 
> Can we tell people developing on Windows
> that hg.bat is buggy and they should use hg.exe?
> There are certainly plenty of other hg bugs we
> tell people to work around (usually by upgrading
> to a different hg).
> 
> Russ

[mattn, 2011-09-21 00:37:11]

> On 2011/09/08 03:32:39, rsc wrote:
> > Can we tell people developing on Windows
> > that hg.bat is buggy and they should use hg.exe?
> 
> SGTM
> 
> Peter

This is not a problem of mercurial or hg.bat.

[mattn, 2011-09-26 06:41:55]

*** Abandoned ***