cannot redirect output about stgit HOT 5 CLOSED

I can certainly understand not wanting to make output scriptable, but I think that's actually somewhat of a user-hostile design choice. I think the prototypical example for many years has been ls which we always advocate against using its output for scripting purposes because of how it can subtly change from BSD to POSIX distributions. ls does not, however, try to hide or obfuscate its output to deter such usages: sometimes you can indeed just simply use a tool incorrectly and suffer the consequences from it.

In this case, I am not actually trying to capture the output for scripting purposes, I'm merely calling stgit from subprocess in another script to apply a mailbox, and simply wish to relay the output to the human user so they have some visual feedback while the series is applying.

(That script is called "patches", a mailbox downloader client written in python we use in the QEMU community for fetching and applying patches sent to our mailing list. I was seeking to modify this script to understand when stgit is present and apply a patch series as a stack instead of using native git commands as an enhancement for my personal workflow, which is stgit-centric.)

I suppose otherwise I can attempt to make calls directly into the stgit library to achieve what I want, but I reasoned I would have the best luck by going through the top-level tools which I believed would be the best-supported, most stable API.

from stgit.

I do not know the full story behind this isatty() policy, but I imagine that at least part of the intent is to prevent stgit's ostensibly human-readable strings from being [ab]used for scripting purposes. I.e. to prevent exactly the use case presented here.

I can empathize with both the original intent behind the isatty() policy and the use case presented here.

If stgit's output string are being used in scripts, then they become part of stgit's interface and thus any changes to these strings may have the side effect of breaking 3rd-party scripts.

On the other hand, for stg import specifically, there is no canonical way to know which patches were found/imported from the import source. Perhaps it could be figured out from parsing stg series before and after, but that's clearly inferior to just getting the list of imported patches directly from stg import.

Perhaps stg import should have a defined output interface for its import and update actions, and call out.stdout() which always outputs to stdout independent of isatty(). In other words, modify imprt.py to unconditionally print to stdout messages in a machine+human readable form that is useful for scripting.

I do not think that globally changing the isatty() semantics of MessagePrinter would be easily acceptable. We would have to identify that every info(), start(), done(), and note() is okay to unconditionally be printed to stdout.

from stgit.

I basically agree with you, it seems broken that a script cannot be written that forwards stg's essential output to its stdout/stderr.

Speaking of stderr, I might claim the problem goes a step deeper in that stgit seems to make the common mistake of reserving stderr for "errors" instead of using stderr for all diagnostic messages. In the case of stg import, I argue that the "Importing patch "x" ..." "done" messages rightly belong on stderr. On the other hand, a command like stg series should be outputting to stdout since the information about the series is its essential output and not a diagnostic.

Keeping the stdout/stderr dichotomy straight might help with both the problem I originally perceived--that its in stgit's interest that diagnostic messages are not parsed by scripts--and with the actual problem at hand of being able to write scripts that can forward diagnostic messages to its stderr while potentially consuming essential output from stdout.

The question for me at this point is how to proceed? I would like stgit's various commands to be audited for stdout/stderr correctness. Any changes would likely trigger test breakages that would need repair. And if we remove the isatty() test to make diagnostic messages unconditional, there might be further fallout in both tests and from stgit users.

All of this is solvable. And perhaps there is an incremental approach that would get stg import repaired sooner than later. Just need someone to carry the torch.

from stgit.

OK, let me see how messy this gets. I can try to do a terminal IO audit.

from stgit.

OK, so the good news is that if I do the stupid thing and delete the isatty() check and see what breaks, only a meager two tests break:

t1004-pack-ref breaks because the number of lines change. It can be changed to simply expect three lines; or we could choose to put the "Available branches" message on stderr instead of stdout.
(This doesn't look like something you want to try and script with stdout anyway, so either approach seems arguably valid...? Thoughts?)

t4000-upgrade.sh breaks because the upgraded notice shows up in stdout. That can be changed by adding something like this:

@@ -113,6 +113,10 @@ class MessagePrinter(object):
         self.__stdout.write_bytes(byte_data)
         self.__stdout.flush()
 
+    def stderr(self, line):
+        """Write a line to stderr."""
+        self.__stderr.write_line(line)

and then changing the upgrade notice:

@@ -59,7 +59,7 @@ def update_to_current_format_version(repository, branch):
             return None
 
     def set_format_version(v):
-        out.info('Upgraded branch %s to format version %d' % (branch, v))
+        out.stderr('Upgraded branch %s to format version %d' % (branch, v))
         config.set(key, '%d' % v)

However, it is likely the case that both info() and note() should semantically be printing to stderr anyway. So, if we just plunder ahead and do that along with removing the isatty() code... Ah! no tests break at all!

There's probably more auditing to do -- the interactive progress messages come to mind at least -- but maybe this is good enough for right now?

from stgit.