Unable to read the output from stream

[spookyd]

I am trying to setup a CI pipeline that will consume the JSON output. Currently the output of this command is not made available to the next command so I am unable to get the JSON. What is odd is that both TerminalController and NinjaProgressAnimation are created using stdoutStream but only NinjaProgressAnimation is available in the output. You can view this behavior by running swift-package-info full-analyzes --for https://github.com/ReactiveX/RxSwift -v 6.0.0 --product RxSwift --report jsonDump >> output.txt.

Here is the output

[1/7] Cleaning up empty app directory...
[2/7] Cloning empty app...
[3/7] Generating archive for empty app...
[4/7] Calculating binary size...
[5/7] Adding RxSwift as dependency...
[6/7] Generating archive for updated app...
[7/7] Calculating updated binary size...

It seems that print does send output to stdout. To validate I created a new type that conforms to TerminalControlling and simply uses print in the write method. Here is the output using the same command as above.

Identified Swift Package:
Repository URL: https://github.com/ReactiveX/RxSwift
Version: 6.0.0
Product: RxSwift
[1/7] Cleaning up empty app directory...
[2/7] Cloning empty app...
[3/7] Generating archive for empty app...
[4/7] Calculating binary size...
[5/7] Adding RxSwift as dependency...
[6/7] Generating archive for updated app...
[7/7] Calculating updated binary size...
{
  "binarySize" : {
    "amount" : 921600,
    "formatted" : "922 KB"
  },
  "dependencies" : [

  ],
  "platforms" : {

  }
}

Any reason not to simply use print or add the option to use print for the output?

[marinofelipe]

Hey @spookyd, very good finding and question 😄 , thanks !

So, taking a quick look at swift-tools-support-core's TerminalController, it has a failable initializer (TerminalController:83) that returns nil when the output of the stream is not a terminal.. which I think should explain why it doesn't output anything to stdout in that case.

Any reason not to simply use print or add the option to use print for the output?

The reason back there was mostly to explore swift-tools-support-core and then TerminalController was also used since it provides some helpers around terminals like colored text, etc.

Perhaps a simple way would be to do a small change inside Console.swift (ie in the init itself Console:127) to fallback to print when TerminalControlling init fails.. which could even be done via a custom implementation of TerminalControlling like you did.

What do you think? I'm also open to other alternatives and suggestions

[spookyd]

I think having a fallback would be a great option as it will guarantee an output.

What conditions would cause a fallback? Obviously, the TerminalController failing to initialize would cause this but what are those conditions? Should there be some sort of output when the fallback occurs?

So, taking a quick look at swift-tools-support-core's TerminalController, it has a failable initializer (TerminalController:83) that returns nil when the output of the stream is not a terminal.. which I think should explain why it doesn't output anything to stdout in that case.

That is an interesting find. Could you explain why this is not able to initialize? I'm curious how is terminal not available if the tool is ran from terminal?

The reason back there was mostly to explore swift-tools-support-core and then TerminalController was also used since it provides some helpers around terminals like colored text, etc.

Learning opportunities are the perfect reason ❤️.

[marinofelipe]

I think having a fallback would be a great option as it will guarantee an output.

Nice 😄

What conditions would cause a fallback? Obviously, the TerminalController failing to initialize would cause this but what are those conditions? Should there be some sort of output when the fallback occurs?

Not that I'm aware of, but I would have to check and confirm that.

Perhaps a strategy here would be to cover the failable initializer, then we can test the case you reported and see if it fixes it? Then after that, in separate, we could further explore if there are other cases where the fallback would come handy. What do you think?

Learning opportunities are the perfect reason ❤️.

Agreed, always 🙌😄

---

That is an interesting find. Could you explain why this is not able to initialize? I'm curious how is terminal not available if the tool is ran from terminal?

To be honest I'm not sure how it works internally, but it seems to use a Darwin function (isatty(:_)) to check if file descriptor of the stream passed to the controller (stdout as you mentioned in this case) is terminal or not.

My hunch was that by piping the stream to a file, it could lead to that method returning false, and I was now able to reproduce it 🙏👇 (though unsure if there could be other cases like mentioned above)

Running without piping the output to a file

.build/debug/swift-package-info binary-size --url https://github.com/marinofelipe/CurrencyText --report

leads to

>>> isTerminal: true

>>> Optional(TSCBasic.TerminalController)
Identified Swift Package:
>>> Optional(TSCBasic.TerminalController)
Repository URL: https://github.com/marinofelipe/CurrencyText
Version: Undefined
Product: Undefined

>>> Optional(TSCBasic.TerminalController)
Invalid version: Undefined

>>> Optional(TSCBasic.TerminalController)
Using latest found tag instead: 3.0.0

>>> Optional(TSCBasic.TerminalController)
Invalid product: Undefined

>>> Optional(TSCBasic.TerminalController)
Using first found product instead: CurrencyTextSwiftUI

[7/7] Calculating updated binary size...
>>> Optional(TSCBasic.TerminalController)
{
  "binarySize" : {
    "amount" : 139264,
    "formatted" : "139 KB"
  }
}

While piping it

.build/debug/swift-package-info binary-size --url https://github.com/marinofelipe/CurrencyText --report jsonDump >> output.txt

leads to

>>> isTerminal: false
>>> nil
>>> nil
>>> nil
>>> nil
>>> nil
>>> nil
[1/7] Cleaning up empty app directory...
[2/7] Cloning empty app...
[3/7] Generating archive for empty app...
[4/7] Calculating binary size...
[5/7] Adding CurrencyTextSwiftUI as dependency...
[6/7] Generating archive for updated app...
[7/7] Calculating updated binary size...
>>> nil

[spookyd]

Perhaps a strategy here would be to cover the failable initializer, then we can test the case you reported and see if it fixes it? Then after that, in separate, we could further explore if there are other cases where the fallback would come handy. What do you think?

I think this is good approach. Is it something you would like contributor help with? Happy to jump in if you would like 😄

---

To be honest I'm not sure how it works internally, but it seems to use a Darwin function (isatty(:_)) to check if file descriptor of the stream passed to the controller (stdout as you mentioned in this case) is terminal or not.

Very interesting information. Thanks for sharing your findings. Wonder if this is a bug in the TSCBasic library itself 🤔

[marinofelipe]

I think this is good approach. Is it something you would like contributor help with? Happy to jump in if you would like 😄

Contributions are always welcomed 😄, so I'd be very happy. And then if you need any help just let me know.

Very interesting information. Thanks for sharing your findings. Wonder if this is a bug in the TSCBasic library itself 🤔

Could be. Maybe it tries to be smart and does that to avoid writing to the terminal when the output stream should be being piped into a file? I really don't know.

[spookyd]

Happy to contribute.

There could be a simple if else check within the init of the Console. Like

        self.isOutputColored = isOutputColored
        if let terminalController = terminalController {
            self.terminalController = terminalController
        } else {
            self.terminalController = PrintController()
        }
        self.progressAnimation = progressAnimation

If you would like to have unit tests for the fallback there could be a factory class and added to the initialization as a default. Like,

    public init(
        isOutputColored: Bool,
        terminalController: TerminalControlling = TerminalControllerFactory.make(),
        progressAnimation: ProgressAnimationProtocol = NinjaProgressAnimation(stream: stdoutStream)
    ) {
        self.isOutputColored = isOutputColored
        self.terminalController = terminalController
        self.progressAnimation = progressAnimation
    }

Do you have a preference?

[spookyd]

I have opened a PR with both approaches I mention above.

39 Simple implementation

40 Same concept just uses a factory type to coordinate the fallback

[marinofelipe]

Hey @spookyd , thanks a lot for the contribution 🙇 , and sorry for the late response, it has been a pretty busy week.

So, I'd personally prefer the second so even though Console is not tested, it would still be testable :).

Great work doing both approaches 🙌 , I will review then Today in a couple hours when I'm back, thanks again 🎉 !!

[spookyd]

Happy to be able to help 😄

[marinofelipe]

The release is out there 🚀 - 1.3.3