[PATCH] git-p4 submit: prevent 'Jobs' section from being removed from p4 change log

[PATCH] git-p4 submit: prevent 'Jobs' section from being removed from p4 change log

[Michael Horowitz]

In an attempt to overwrite the 'Description:' section of the p4 change
log to include the git commit messages, it also overwrote the 'Jobs:'
section.  This fix restores the 'Job:' section.

Signed-off-by: Michael Horowitz <michael.horowitz@ieee.org>
---
 contrib/fast-import/git-p4 |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index a92beb6..8b00fd8 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -570,7 +570,7 @@ class P4Submit(Command):
                continue

            if inDescriptionSection:
-                if line.startswith("Files:"):
+                if line.startswith("Files:") or line.startswith("Jobs:"):
                    inDescriptionSection = False
                else:
                    continue
--
1.7.4

Re: [PATCH] git-p4 submit: prevent 'Jobs' section from being removed from p4 change log

[Junio C Hamano]

Michael Horowitz <michael.horowitz@ieee.org> writes:

> In an attempt to overwrite the 'Description:' section of the p4 change
> log to include the git commit messages, it also overwrote the 'Jobs:'
> section.  This fix restores the 'Job:' section.
>
> Signed-off-by: Michael Horowitz <michael.horowitz@ieee.org>
> ---
>  contrib/fast-import/git-p4 |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index a92beb6..8b00fd8 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -570,7 +570,7 @@ class P4Submit(Command):
>                 continue
>
>             if inDescriptionSection:
> -                if line.startswith("Files:"):
> +                if line.startswith("Files:") or line.startswith("Jobs:"):
>                     inDescriptionSection = False
>                 else:
>                     continue

This is not a new issue with the code, but it makes me wonder if the
output you are reading from guaranteed to have these lines in the same
order.  Otherwise the next bug report and/or patch would add another
similar looking line.startswith("SomethingElse:") to this statement, and
we wouldn't know when to stop, would we?

Will queue anyway, though.  Thanks.

Re: [PATCH] git-p4 submit: prevent 'Jobs' section from being removed from p4 change log

[Michael Horowitz]

On Sat, Feb 26, 2011 at 2:37 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Michael Horowitz <michael.horowitz@ieee.org> writes:
>
>> In an attempt to overwrite the 'Description:' section of the p4 change
>> log to include the git commit messages, it also overwrote the 'Jobs:'
>> section.  This fix restores the 'Job:' section.
>>
>> Signed-off-by: Michael Horowitz <michael.horowitz@ieee.org>
>> ---
>>  contrib/fast-import/git-p4 |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
>> index a92beb6..8b00fd8 100755
>> --- a/contrib/fast-import/git-p4
>> +++ b/contrib/fast-import/git-p4
>> @@ -570,7 +570,7 @@ class P4Submit(Command):
>>                 continue
>>
>>             if inDescriptionSection:
>> -                if line.startswith("Files:"):
>> +                if line.startswith("Files:") or line.startswith("Jobs:"):
>>                     inDescriptionSection = False
>>                 else:
>>                     continue
>
> This is not a new issue with the code, but it makes me wonder if the
> output you are reading from guaranteed to have these lines in the same
> order.  Otherwise the next bug report and/or patch would add another
> similar looking line.startswith("SomethingElse:") to this statement, and
> we wouldn't know when to stop, would we?
>
> Will queue anyway, though.  Thanks.
>
>

Yes, you are correct, it could be written in a more robust way.
Ideally, with a proper spec, the parser can be written to handle all
the cases.  Unfortunately, I am not familiar enough to do much more
than fix the immediate issue I am having.  I only know enough Python
to make this simple change, and it seems to work in my tests.

Thanks,

Mike