From: Karthik Nayak <karthik.188@gmail.com>
To: Fahad Alrashed via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org
Cc: Fahad Alrashed <fahad@keylock.net>
Subject: Re: [PATCH] Bug fix: ensure P4 "err" is displayed when exception is raised.
Date: Mon, 6 May 2024 05:01:45 -0700 [thread overview]
Message-ID: <CAOLa=ZQUEHO0oCcccgHQqzvgXkK-gMOqhpCQ2fGTLLMBPB+Pag@mail.gmail.com> (raw)
In-Reply-To: <pull.1668.git.git.1714802221671.gitgitgadget@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3544 bytes --]
Hello Fahad,
"Fahad Alrashed via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Fahad Alrashed <fahad@keylock.net>
>
> Bug fix: During "git p4 clone" if p4 process returns
Nit: I haven't seen us use a prefix for the commit message. Also we use
a max line length of 72 characters (see .editorconfig). The commit
message seems to be wrapped at a much lower threshold.
> an error from the server, it will store it in variable
perhaps `in the 'err' variable` or `in a variable 'err'`.
> "err". The it will send a text command "die-now" to
s/The/Then
> git-fast-import. However, git-fast-import raises an
> exception: "fatal: Unsupported command: die-now"
> and err is never displayed. This patch ensures that
> err is dispayed using "finally:"
>
> Signed-off-by: Fahad Alrashed <fahad@keylock.net>
> ---
> In git p4, git fast-import fails from die-now command and err (from
> Perforce) is not shown
>
> When importing from Perforce using git p4 clone <depot location>,
> cloning works fine until Perforce command p4 raises an error. This error
> message is stored in err variable then git-fast-import is sent a die-now
> command to kill it. An exception is raised fatal: Unsupported command:
> die-now.
>
> This patch forces python to call die() with the err message returned
> from Perforce.
>
> This commit fixes the root cause of a bug that took me hours to find.
> I'm sure many faced the cryptic error and declared that git-p4 is not
> working for them.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1668%2Falrashedf%2Fmaster-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1668/alrashedf/master-v1
> Pull-Request: https://github.com/git/git/pull/1668
>
> git-p4.py | 24 +++++++++++++-----------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index 28ab12c72b6..f1ab31d5403 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -3253,17 +3253,19 @@ def streamP4FilesCb(self, marshalled):
> if self.stream_have_file_info:
> if "depotFile" in self.stream_file:
> f = self.stream_file["depotFile"]
> - # force a failure in fast-import, else an empty
> - # commit will be made
> - self.gitStream.write("\n")
> - self.gitStream.write("die-now\n")
> - self.gitStream.close()
> - # ignore errors, but make sure it exits first
> - self.importProcess.wait()
> - if f:
> - die("Error from p4 print for %s: %s" % (f, err))
> - else:
> - die("Error from p4 print: %s" % err)
> + try:
> + # force a failure in fast-import, else an empty
> + # commit will be made
> + self.gitStream.write("\n")
> + self.gitStream.write("die-now\n")
> + self.gitStream.close()
> + # ignore errors, but make sure it exits first
> + self.importProcess.wait()
> + finally:
> + if f:
> + die("Error from p4 print for %s: %s" % (f, err))
> + else:
> + die("Error from p4 print: %s" % err)
>
This part looks good.
> if 'depotFile' in marshalled and self.stream_have_file_info:
> # start of a new file - output the old one first
>
> base-commit: 235986be822c9f8689be2e9a0b7804d0b1b6d821
> --
> gitgitgadget
Thanks,
Karthik
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
next prev parent reply other threads:[~2024-05-06 12:01 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-04 5:57 [PATCH] Bug fix: ensure P4 "err" is displayed when exception is raised Fahad Alrashed via GitGitGadget
2024-05-06 12:01 ` Karthik Nayak [this message]
2024-05-08 11:46 ` [PATCH v2] " Fahad Alrashed via GitGitGadget
2024-05-08 16:53 ` Junio C Hamano
2024-05-08 22:11 ` [PATCH v3] git-p4: show Perforce error to the user Fahad Alrashed via GitGitGadget
2024-05-09 15:30 ` 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='CAOLa=ZQUEHO0oCcccgHQqzvgXkK-gMOqhpCQ2fGTLLMBPB+Pag@mail.gmail.com' \
--to=karthik.188@gmail.com \
--cc=fahad@keylock.net \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
/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.