From: Junio C Hamano <gitster@pobox.com>
To: Luke Diamand <luke@diamand.org>
Cc: git <git@vger.kernel.org>
Subject: Re: git-p4: test t9820-git-p4-editor-handling.sh failing
Date: Tue, 19 May 2015 08:44:53 -0700 [thread overview]
Message-ID: <xmqqa8x0a7wq.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <555ADAF7.5020001@diamand.org> (Luke Diamand's message of "Tue, 19 May 2015 07:40:55 +0100")
Luke Diamand <luke@diamand.org> writes:
> The test I put in recently for multi-word editor handling in git-p4,
> t9820-git-p4-editor-handling.sh, has started failing.
>
> It looks like the reason is the change to it that goes:
>
> - P4EDITOR="touch \"$git/touched\"" git p4 submit &&
> + P4EDITOR=": >\"$git/touched\"" git p4 submit &&
>
> The problem is that git-p4 invokes $P4EDITOR passing it the name of
> the submit template. After it returns, it checks that the editor has
> actually updated the file's modification time.
Sorry, that was an unwarranted and unnecessary amend. Didn't
realize that touch was trying to affect two files.
But "touch" is not quite right, either.
Unlike human sitting in front of keyboard, our fake editor types
very fast and wallclock time may not change between the time when
"git p4" prepares the file to be edited and the fake editor returns.
Is it really *only* the modification time that is checked? If our
fake editor adds one blank line and return very fast without
changing the modification time, doesn't the caller notice that (and
if not, shouldn't it be fixed to do so [*1*])?
If you absolutely need to change the timestamp to work around the
caller if it only checks the timestamp and does not notice the size
or contents are different, then test-chmtime would be the right
thing to use in the test suite to do this portably, something like.
P4EDITOR=": >\"$git/touched\" && test-chmtime +5"
perhaps.
Thanks.
[Footnote]
*1* Yeah, I just checked. It does check only mtime and wants the
editor to spend at least one second to edit, which is silly X-<.
next prev parent reply other threads:[~2015-05-19 15:45 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-19 6:40 git-p4: test t9820-git-p4-editor-handling.sh failing Luke Diamand
2015-05-19 15:44 ` Junio C Hamano [this message]
2015-05-19 22:23 ` [PATCHv3 0/3] git-p4: fix handling of multi-word P4EDITOR Luke Diamand
2015-05-19 22:23 ` [PATCHv3 1/3] git-p4: add failing test for P4EDITOR handling Luke Diamand
2015-05-20 19:54 ` Junio C Hamano
2015-05-20 20:56 ` Junio C Hamano
2015-05-20 21:43 ` Luke Diamand
2015-05-19 22:23 ` [PATCHv3 2/3] git-p4: fix handling of multi-word P4EDITOR Luke Diamand
2015-05-19 22:23 ` [PATCHv3 3/3] git-p4: tests: use test-chmtime in place of touch Luke Diamand
2015-05-19 22:36 ` Luke Diamand
2015-05-24 18:56 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqa8x0a7wq.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=luke@diamand.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.