cccmd no longer seems to be invoked

[andreabolognani]

I tried sending some QEMU patches today and the CC list was suspiciously empty.

Running git format-patch --no-cover-letter master followed by scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null *.patch resulted in a list of email addresses.

I'm running git-publish on Fedora 35, installed from RPM package. There was an update to 1.8.0 on 2022-04-25, and the previous time I sent out QEMU patches on 2022-04-20 everything worked fine.

I tried running git-publish 1.8.0 from a git clone, same behavior. I reverted back to 1.7.0 and the CC list is once again populated.

7155b54 changes how cccmd is invoked, so that'd be my first guess at a culprit.

[stefano-garzarella]

7155b54 changes how cccmd is invoked, so that'd be my first guess at a culprit.

Yep, I had the same issue and your conclusion is right!

The problem is that QEMU's .gitpublish contains 2>/dev/null that doesn't work well with subprocess.Popen():

Popen input = ['scripts/get_maintainer.pl', '--noroles', '--norolestats', '--nogit', '--nogit-fallback', '2>/dev/null', '/tmp/tmpktz5gmr3/0001-TEST.patch']
cmd = <Popen: returncode: None args: ['scripts/get_maintainer.pl', '--noroles', '-...>
popen_lines(cmd) = ([], ["scripts/get_maintainer.pl: file '2>/dev/null' not found: No such file or directory"])

Removing 2>/dev/null from .gitpublish should work, but we should fix git-publish IMHO.

The following patch fixes the issue for me (on Fedora 35), but I don't know if it reintroduces the issue fixed by 7155b54 on Windows:

diff --git a/git-publish b/git-publish
index 0ed375a..1ca91d3 100755
--- a/git-publish
+++ b/git-publish
@@ -902,12 +902,9 @@ branch.%s.pushRemote is set appropriately?  (Override with --no-check-url)''' %
                     # The encoding of cc-cmd output is not well-defined. Use git's encoding for now
                     # although git-send-email is a Perl script that uses Perl's Unicode support rather
                     # than git's standard UTF-8 encoding.
-                    cmd = subprocess.Popen(shlex.split(cc_cmd + " " + x),
-                           stdout=subprocess.PIPE,
-                           stderr=subprocess.PIPE,
-                           cwd=git_get_toplevel_dir())
-                    output, _ = popen_lines(cmd)
-                    cc = cc.union(output)
+                    output = subprocess.check_output(cc_cmd + " " + x,
+                    shell=True, cwd=git_get_toplevel_dir()).decode(GIT_ENCODING)
+                    cc = cc.union(output.splitlines())
             cc.difference_update(to)
             if inspect_emails:
                 selected_patches = inspect_menu(tmpdir, to, cc, patches, suppress_cc,

@lygstate What's the reason why we can't use subprocess.check_output()?

[lygstate]

7155b54 changes how cccmd is invoked, so that'd be my first guess at a culprit.

Yep, I had the same issue and your conclusion is right!

The problem is that QEMU's .gitpublish contains 2>/dev/null that doesn't work well with subprocess.Popen():

Popen input = ['scripts/get_maintainer.pl', '--noroles', '--norolestats', '--nogit', '--nogit-fallback', '2>/dev/null', '/tmp/tmpktz5gmr3/0001-TEST.patch']
cmd = <Popen: returncode: None args: ['scripts/get_maintainer.pl', '--noroles', '-...>
popen_lines(cmd) = ([], ["scripts/get_maintainer.pl: file '2>/dev/null' not found: No such file or directory"])

Removing 2>/dev/null from .gitpublish should work, but we should fix git-publish IMHO.

The following patch fixes the issue for me (on Fedora 35), but I don't know if it reintroduces the issue fixed by 7155b54 on Windows:

diff --git a/git-publish b/git-publish
index 0ed375a..1ca91d3 100755
--- a/git-publish
+++ b/git-publish
@@ -902,12 +902,9 @@ branch.%s.pushRemote is set appropriately?  (Override with --no-check-url)''' %
                     # The encoding of cc-cmd output is not well-defined. Use git's encoding for now
                     # although git-send-email is a Perl script that uses Perl's Unicode support rather
                     # than git's standard UTF-8 encoding.
-                    cmd = subprocess.Popen(shlex.split(cc_cmd + " " + x),
-                           stdout=subprocess.PIPE,
-                           stderr=subprocess.PIPE,
-                           cwd=git_get_toplevel_dir())
-                    output, _ = popen_lines(cmd)
-                    cc = cc.union(output)
+                    output = subprocess.check_output(cc_cmd + " " + x,
+                    shell=True, cwd=git_get_toplevel_dir()).decode(GIT_ENCODING)
+                    cc = cc.union(output.splitlines())
             cc.difference_update(to)
             if inspect_emails:
                 selected_patches = inspect_menu(tmpdir, to, cc, patches, suppress_cc,

@lygstate What's the reason why we can't use subprocess.check_output()?

I am using the popen_lines for decoding git output properly. So the issue is shlex.split doesn't support for 2>/dev/null pipe redirection?

[stefano-garzarella]

I am using the popen_lines for decoding git output properly. So the issue is shlex.split doesn't support for 2>/dev/null pipe redirection?

Nope, shlex.split() should only split the string. Instead subprocess.Popen() doesn't support 2>/dev/null well.

Can you try if the above patch works on Windows as well?

[lygstate]

OK, lets' me try

[stefanha]

@stefano-garzarella Your patch looks good to me. Once @lygstate has confirmed it works on Windows I'll merge it and release git-publish 1.8.1 with the fix.

[lygstate]

Tried, it's works