Merge lp://qastaging/~elopio/snapcraft/fix1476452-python_log into lp://qastaging/~snappy-dev/snapcraft/core

Thanks for this branch. I like it and code-wise it looks great, just two comments inline (which are the same) where a small comment might be helpful. I +1 on this branch but I will leave the top-approve to Mike.

A) This has conflicts now

B) Does this keep the bold text output? I thought that was rather valuable for separating the non-bold output of plugin subprocesses from the bold output of snapcraft itself (bracketing plugin output when going through the stages).

I'd prefer to keep that if possible.

> A) This has conflicts now

and it requires sergio to add fixtures to the machine :(

> B) Does this keep the bold text output? I thought that was rather valuable
> for separating the non-bold output of plugin subprocesses from the bold output
> of snapcraft itself (bracketing plugin output when going through the stages).
>
> I'd prefer to keep that if possible.

No, it doesn't. I can do it with with separate handlers and filters, I think. Let me try.

> Thanks for this branch. I like it and code-wise it looks great, just two
> comments inline (which are the same) where a small comment might be helpful. I
> +1 on this branch but I will leave the top-approve to Mike.

Thanks for the review. I added comments on all the checks for the return code.

> A) This has conflicts now

Merged and all tests passing.

> B) Does this keep the bold text output? I thought that was rather valuable
> for separating the non-bold output of plugin subprocesses from the bold output
> of snapcraft itself (bracketing plugin output when going through the stages).
>
> I'd prefer to keep that if possible.

Now the logger format has the bold wrapper. Please check if that's what you wanted.

From IRC:

<mterry> elopio, looking at your branch -- now some things are bold that weren't before
<mterry> elopio, look at the diff in LP and you can see some things were just print() lines before
 elopio, specifically when we ran a subcommand, it would not be bold
 elopio, and a couple tiny other places
 elopio, what's the cleanest way to distinguish there? Use another logger command than info?

> From IRC:
>
>
> <mterry> elopio, looking at your branch -- now some things are bold that
> weren't before
> <mterry> elopio, look at the diff in LP and you can see some things were just
> print() lines before
> elopio, specifically when we ran a subcommand, it would not be bold
> elopio, and a couple tiny other places
> elopio, what's the cleanest way to distinguish there? Use another logger
> command than info?l

No, I think the correct way is to define a different logger, like:
subcommand_logger = logging.getLogger(__name__ + '.subcommand')

And then we add a handler to that subcommand_logger that has a formatter without the bold wrapper.
However, if you see at the statements that use print:

66 - print(' '.join(cmd))

141 - snapcraft.common.log("Wrote the following as snapcraft.yaml.")
142 - print()
143 - print(yaml)

170 - print("Installing required packages on the host system: " + ", ".join(newPackages))

I fail to see a pattern in there that we can generalize in a logger. So maybe instead, each call to the logger should decide if it should be wrapped in bold or not. And IMO, instead of using \bold it would be clearer to use the name of the package. Like:

snapcraft log message-1
plugin.ubuntu: ubuntu log message-1
...
plugin.ubuntu: ubuntu log message-n
snapcraft log message-last

In the end, I think we should define a logging policy so we can be consistent on how to use it, and then define the logger hierarchy so it's easy to use. For now, I'll revert those lines to use print so this branch will introduce the loggers while keeping the exact same behaviour as before. And lets start the discussion.

Looks good. We can go over the bold/not-bold stuff separately. I'm sure there's a nicer way to distinguish those.