Re: [PATCH v1] git-p4: Add option to ignore empty commits

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 20, 2015, 17:27 UTC
Message-ID: <xmqqmvvd8n7t.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1445280239-39840-1-git-send-email-larsxschneider@gmail.com>

larsxschneider@gmail.com writes:

Show 17 quoted lines

> diff --git a/git-p4.py b/git-p4.py
> index 0093fa3..6c50c74 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -2288,12 +2288,6 @@ class P4Sync(Command, P4UserMap):
>          filesToDelete = []
>  
>          for f in files:
> -            # if using a client spec, only add the files that have
> -            # a path in the client
> -            if self.clientSpecDirs:
> -                if self.clientSpecDirs.map_in_client(f['path']) == "":
> -                    continue
> -
>              filesForCommit.append(f)
>              if f['action'] in self.delete_actions:
>                  filesToDelete.append(f)

Earlier, the paths outside the clientspec were not in filesToDelete (or filesToRead that is below the context here). Now they all go to these arrays, and will hit this loop beyond the context:

        # deleted files...
        for f in filesToDelete:
            self.streamOneP4Deletion(f)

after leaving the above for loop. I cannot quite see where this "stream one deletion" is turned into a no-op for paths outside after this patch gets applied.

Also I have this suspicion that those who do want to use client spec to get a narrowed view into the history would almost always want this "ignore empty" behaviour (I'd even say the current behaviour to leave empty commits by default is a bug). What are the advantages of keeping empty commits? If there aren't many, perhaps git-p4 should by the default skip empties and require p4.keepEmpty configuration to keep them?