Re: [PATCH] Wait for git diff to finish in git difftool

Alex Riesen [off-list ref] writes:

2009/4/23 Junio C Hamano [off-list ref]:

quoted

Alex Riesen [off-list ref] writes:

quoted

Frankly, I just always forget about it. The patches to my windows
repository are mostly just hacks to either make it pass test suite or
workaround another, the platforms specific, stupidity. Not sure anyone
is actually interested in them (because, I repeat, they mostly hurt
everyone else, while making the platforms port "a little working").

What do you really mean?  These patches add maintenance burden but its
benefit nobody would care about, and should not be applied?

Maintenance _and_ run-time. It is your decision, but yes, I kind of hate
the idea of them being applied.

In plain words: please don't apply MY patches with words "Windows"
or "Cygwin" in message body, unless I beg you to (and even then,
consider me heavily drugged).

Hmm, what's the point then for me to spend time looking at them posted on
the list?

2009/4/23 Junio C Hamano [off-list ref]:

Alex Riesen [off-list ref] writes:

quoted

2009/4/23 Junio C Hamano [off-list ref]:

quoted

Alex Riesen [off-list ref] writes:

quoted

Frankly, I just always forget about it. The patches to my windows
repository are mostly just hacks to either make it pass test suite or
workaround another, the platforms specific, stupidity. Not sure anyone
is actually interested in them (because, I repeat, they mostly hurt
everyone else, while making the platforms port "a little working").

What do you really mean?  These patches add maintenance burden but its
benefit nobody would care about, and should not be applied?

Maintenance _and_ run-time. It is your decision, but yes, I kind of hate
the idea of them being applied.

In plain words: please don't apply MY patches with words "Windows"
or "Cygwin" in message body, unless I beg you to (and even then,
consider me heavily drugged).

Hmm, what's the point then for me to spend time looking at them posted on
the list?

None. But it still would be nice if you look at them when you're addressed
directly. Please understand, maybe I'm abusing the list a little, but it looks
like the best place to _archive_ the workarounds for this obscure platform.
The code is of little use to general public, but in case one finds himself
in the same situation as I am in, at least Google can help out. I think it
happens too rare to justify your looking at them, and especially including
them in the mainline, but hiding the code on my laptop isn't of any use
to anyone (including me: I don't always have that laptop).

Heya,

On Thu, Apr 23, 2009 at 11:52, Alex Riesen [off-list ref] wrote:

None. But it still would be nice if you look at them when you're addressed
directly. Please understand, maybe I'm abusing the list a little, but it looks
like the best place to _archive_ the workarounds for this obscure platform.

In that case you should note in your patch (preferrably in the
subject) that they are not meant for inclusion and merely for the that
purpose so that those who are not interested in them need not waste
their time reviewing the patch with the assumption that it _is_ meant
for inclusion.

-- 
Cheers,

Sverre Rabbelier

Alex Riesen [off-list ref] writes:

quoted

quoted

In plain words: please don't apply MY patches with words "Windows"
or "Cygwin" in message body, unless I beg you to (and even then,
consider me heavily drugged).

Hmm, what's the point then for me to spend time looking at them posted on
the list?

None. But it still would be nice if you look at them when you're addressed
directly. Please understand, maybe I'm abusing the list a little, but it looks
like the best place to _archive_ the workarounds for this obscure platform.
The code is of little use to general public, but in case one finds himself
in the same situation as I am in, at least Google can help out. I think it
happens too rare to justify your looking at them, and especially including
them in the mainline, but hiding the code on my laptop isn't of any use
to anyone (including me: I don't always have that laptop).

Actually, I think I probably should have phrased it more positively.

Do you think you are in a so minority situation that your "workarounds"
are valuable for nobody else?  Apparently you see value in letting them
finding your patches in the archive, so you do not expect them to be
"nobody else" but rather "a small minority".

People on minority platforms have smaller number of people in similar
situation to ask help from than people on mainstream platforms, and they
suffer from the minority status of their platforms not only with git but
about many other things.  Worse, their chance of finding solution by
digging the archive is smaller, and they would need to spend a lot more
time, than the people on other platforms.  And you know this a lot better
than I do ;-).

Even though the suffering of these people as a whole may not be much, only
because there are small number of them, as individuals they may suffer a
lot.  If we can help them without having to bend over backwards too much,
it would be a good thing.  I think we should at least try.

For example, I do not think conversion from Perl exec() to Perl system()
is worsening the code in any way in difftool.  Sure, it won't *improve*
things for general majority of people, but as long as it does not hurt
people (that includes me and others who pay maintenance cost), I do not
think we should just bury the patch in archive or in your laptop.

Junio C Hamano, Thu, Apr 23, 2009 16:51:28 +0200:

Do you think you are in a so minority situation that your "workarounds"
are valuable for nobody else?  Apparently you see value in letting them
finding your patches in the archive, so you do not expect them to be
"nobody else" but rather "a small minority".

Correct, "small minority".

People on minority platforms have smaller number of people in similar
situation to ask help from than people on mainstream platforms, and they
suffer from the minority status of their platforms not only with git but
about many other things.  Worse, their chance of finding solution by
digging the archive is smaller, and they would need to spend a lot more
time, than the people on other platforms.  And you know this a lot better
than I do ;-).

Well, this particular minority is used to have marginal levels of
technical support and .

For example, I do not think conversion from Perl exec() to Perl system()
is worsening the code in any way in difftool.  Sure, it won't *improve*
things for general majority of people, but as long as it does not hurt
people (that includes me and others who pay maintenance cost), I do not
think we should just bury the patch in archive or in your laptop.

Isn't it clearly marked in the archives with the keywords most often
associated with the platform? (they may seem a little... unprintable)

Still, you are, as usual, right. Especially regarding this particular case.

I'll add this comment regarding use of system in the case where exec
is right choice on all accounts and resend the patch:

    # ActiveState Perl for Win32 does not implement POSIX semantics of
    # exec* system call. It just spawns the given executable and finishes
    # the starting program, exiting with code 0.
    # system will at least catch the errors in returned by git diff,
    # allowing the caller of git difftool better handling of failures.

Alex Riesen, Thu, Apr 23, 2009 20:57:32 +0200:

Junio C Hamano, Thu, Apr 23, 2009 16:51:28 +0200:

quoted

For example, I do not think conversion from Perl exec() to Perl system()
is worsening the code in any way in difftool.  Sure, it won't *improve*
things for general majority of people, but as long as it does not hurt
people (that includes me and others who pay maintenance cost), I do not
think we should just bury the patch in archive or in your laptop.

Isn't it clearly marked in the archives with the keywords most often
associated with the platform? (they may seem a little... unprintable)

Still, you are, as usual, right. Especially regarding this particular case.

I'll add this comment regarding use of system in the case where exec
is right choice on all accounts and resend the patch:

    # ActiveState Perl for Win32 does not implement POSIX semantics of
    # exec* system call. It just spawns the given executable and finishes
    # the starting program, exiting with code 0.
    # system will at least catch the errors in returned by git diff,
    # allowing the caller of git difftool better handling of failures.

Oh... I'm too late...

Portability reasons.

Signed-off-by: Alex Riesen <redacted>
---

Alex Riesen, Thu, Apr 23, 2009 21:08:35 +0200:

Alex Riesen, Thu, Apr 23, 2009 20:57:32 +0200:

quoted

I'll add this comment regarding use of system in the case where exec
is right choice on all accounts and resend the patch:

    # ActiveState Perl for Win32 does not implement POSIX semantics of
    # exec* system call. It just spawns the given executable and finishes
    # the starting program, exiting with code 0.
    # system will at least catch the errors in returned by git diff,
    # allowing the caller of git difftool better handling of failures.

Oh... I'm too late...

There.

 git-difftool.perl |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/git-difftool.perl b/git-difftool.perl
index bd828c2..9255d23 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl

@@ -82,5 +82,11 @@ sub generate_command
 }
 
 setup_environment();
+
+# ActiveState Perl for Win32 does not implement POSIX semantics of
+# exec* system call. It just spawns the given executable and finishes
+# the starting program, exiting with code 0.
+# system will at least catch the errors returned by git diff,
+# allowing the caller of git difftool better handling of failures.
 my $rc = system(generate_command());
 exit($rc | ($rc >> 8));

-- 
1.6.3.rc1.74.g42ff