Re: [PATCH] git-p4: fix fast import when empty commit is first

>
> "Alisha Kim via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Alisha Kim <pril@pril.cc>
> >
> > When executing p4 sync by specifying an excluded path, an empty commit
> > will be created if there is only a change in the excluded path in
> > revision.
> > If git-p4.keepEmptyCommits is turned off and an empty commit is the
> > first, fast-import will fail.
>
> The above describe under what condition a failure gets triggered,
> but there is no description on what approach the proposed solution
> takes.  You could teach "fast-import" to deal with an empty commit
> properly, you could ignore empty commits and not produce input for
> the fast-import command, you could probably turn these initial empty
> commits into non-empty commits by adding dummy contents, etc.  We
> want to see in our proposed log messages what solution was taken and
> how the solution was designed to satisfy what requirements.  This is
> to help future developers who will have to change the code that is
> given by this patch, so that their updates can still adhere to what
> ever design criteria you had in working on this change [*].
>
>     Side note: Your solution might be to ignore empty commits
>     despite keepEmptyCommits option is set (as I said, you did not
>     describe it at all in the above, so this is a hypothetical
>     example).  If the reason behind choosing that design were "I
>     just do not want it to fail---I do not care if the resulting
>     history coming out of fast-import is crappy (I lose the p4 CL
>     descriptions for these commits, even though the user wants to
>     keep them)", then future developers can safely "fix" your fix
>     here by turning the initial empty commits into non-empty ones by
>     adding fake contents.

>
>
> > @@ -3876,10 +3878,12 @@ class P4Sync(Command, P4UserMap):
> >                              self.commit(description, filesForCommit, branch, parent)
> >                  else:
> >                      files = self.extractFilesFromCommit(description)
> > -                    self.commit(description, files, self.branch,
> > +                    isCommitted = self.commit(description, files, self.branch,
> >                                  self.initialParent)
> >                      # only needed once, to connect to the previous commit
> > -                    self.initialParent = ""
> > +                    if isCommitted:
> > +                        self.initialParent = ""
>
> "is" does not sound grammatically correct.  "didCommit" (i.e., "we
> made a commit"), "haveCommitted" (i.e., "we have made a commit")
> might be more understandable.

>
>
> >              except IOError:
> >                  print(self.gitError.read())
> >                  sys.exit(1)
> >
> > base-commit: cfb8a6e9a93adbe81efca66e6110c9b4d2e57169