Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is this project dead? #360

Open
ianfixes opened this issue Aug 20, 2019 · 19 comments
Open

Is this project dead? #360

ianfixes opened this issue Aug 20, 2019 · 19 comments

Comments

@ianfixes
Copy link

The last time a pull request was merged in this project was (as of this writing) almost exactly a year ago -- #335. The list of contributors doesn't show anything promising either, and their names don't seem to be among the comments in the most recently-updated pull requests.

Has the project been abandoned?

@jonnermut
Copy link

jonnermut commented Aug 22, 2019

Looks it. But I'm seeing a lot of activity over here: https://github.com/Lightricks/xcpretty

@byohay @barakwei you look as though you are doing some maintenance on xcpretty, are you planning on putting out a release? Any chance we could get #345 or something similar merged into it if so?

@barakwei
Copy link

At this stage, we don't plan on releasing our own xpretty as a gem, nor do we currently accept external pull requests, since we tailor our fork to our needs.
Specifically, regarding the PR mentioned, I guess what you want to do is disable warning via xcodebuild parameters (GCC_WARN_INHIBIT_ALL_WARNINGS sounds useful), instead of hacking it with xcpretty.

Since we maintain our own fork we can point the xcpretty gem to our git repo. In the Gemfile we add gem "xcpretty", git:"https://github.com/Lightricks/xcpretty", so our gem would be used instead of the original one.
Using this method, you can always use your own fork with the commits you need, feel free to use our fork if that suits your needs.

If you're planning to use our fork, notice we've removed some features like custom formatters/reports and we intend to remove reports completely.

@jonnermut
Copy link

jonnermut commented Aug 22, 2019

@barakwei thanks for the info.

Specifically, regarding the PR mentioned, I guess what you want to do is disable warning via xcodebuild parameters (GCC_WARN_INHIBIT_ALL_WARNINGS sounds useful), instead of hacking it with xcpretty.

If xcodebuild played that nicely xcpretty would have zero reason for existence... my current warnings I want to silence are out of the linker, and swift, which you can't silence or at least last time I looked

@barakwei
Copy link

I see. I guess we would accept pull requests that are highly requested and have tests.

@ianfixes
Copy link
Author

ianfixes commented Aug 22, 2019

That's a kind gesture @barakwei, but I think the solution here shouldn't involve simply "failing over" to an individual fork just because there happens to be activity on it. As you say, you're taking xcpretty in your own direction, and I don't think it's necessarily fair or right to now burden you with maintenance tasks for the mainline version that you've already left behind (design-wise).

What I'm hoping to do here is attract the attention of the maintainers to add some new faces to the github org such that the project can continue to evolve. Or, to declare that the project is abandoned -- that we should come up with alternate plans for sorting out all the pull request contributions.

@barakwei
Copy link

Sure thing. I do recommend using our fork since the mainline version is very old and doesn't support many new commands and the new build system. Also, we have some bugfixes and performance improvements.

@supermarin
Copy link
Contributor

Since nobody else has responded to address this issue, here’s my 2 cents:

Since we’re using an in-house solution nowadays (and I don’t work in infra anymore), I don’t have time to keep up with Apple‘s changes and maintain the project. The project has always had > 1 contributors with commit access, but the truth is nobody has time and energy to properly maintain it if they’re not using it themselves.

Since xcpretty is used in some bigger & more popular projects as a dependency, I’ve transferred the ownership as an alternative to shutting it down. As there’s no active maintainers to keep it up and running, IMO it’d be the best to deprecate it and move on with life.

If there’s enough people like @barakwei that would like to take full ownership onwards, let me know and you’ll get commit access.

Are you “pulling an Apple”?

tl;dr - No. There’s better solutions nowadays.

This project was started with a goal to reduce the amount of verbose xcodebuild output, and bring important lines forward to the user. At the time, xcodebuild was mixing stderr and stdout outputs, wasn’t consistent with printing different build stages, and build logs were sometimes over 10MB.
As a side effect of writing a universal parser and a library, we were able to make various sub-parsers and reporters (like junit, json_compilation_database, etc). There’s no question the tool was useful and the right solution at the time, but things have changed.

Since then, Apple has introduced the -quiet flag (do not print any output except for warnings and errors). You should absolutely use that solution for your build machines.

Xcode is also dumping various XML reports (e.g. for test runs), and there’s many tools like Fastlane’s trainer that can post process the test runs and print results nicely. It shouldn’t be hard to write your own micro processing tool when the data you have is structured.

That’s OK, but we still need xcpretty for xyz

Like mentioned before, leave comments below and I’ll happily consider letting more maintainers in.
The branch where I left over before stopping is blacklist. All the work should be done there and it should be merged to master. It is a completely different approach (blacklisting output we know vs whitelisting output we know and suppressing everything else). Parser bits are written in a declarative way and I’m happy to run over whoever wants to pick it up from there.

@ianfixes
Copy link
Author

ianfixes commented Oct 3, 2019

Is there a reason to not just hand out commit access to the people you feel are qualified to make edits?

I can't think of a downside to having more people in a position to merge pull requests; there is a functioning CI and test suite, old versions of this project will always be available if anything goes wrong, and in the event that something does go off the rails, whatever focus that brings to this project would likely result in the bugs being resolved (and more people contributing).

@supermarin
Copy link
Contributor

Not sure if it was clear from my previous comment, but there’s a few people with commit access in the repo (3 directly in xcpretty/xcpretty plus 4 via xcpretty org). This means that people were given access before, and lost interest over time. When that happens, all the issues, mentions etc default back to me.

Adding yourself to maintainers is an option I’ve mentioned above.
I just don’t have the time & energy to keep maintaining people, comms, testing changes, pushing releases etc.

@supermarin
Copy link
Contributor

Sorry if the above comment sounded harsh, dumped my thoughts without much thinking.
Anyways - let me know if you want commit access and have energy to keep pushing the project forward.

@ianfixes
Copy link
Author

I've been thinking about this and I think I'm game to take this on. Is there a doc somewhere about "things we tried that were definitely bad ideas" and/or "purposeful design choices that should be preserved"?

@supermarin
Copy link
Contributor

Sorry for the delay Ian.

I can write up a doc and do a hangout to walk you through everything.
Is later this week OK?

@ianfixes
Copy link
Author

There's no rush; plenty to keep me busy on my end. I'll stay tuned.

@supermarin
Copy link
Contributor

supermarin commented Apr 18, 2020

@ianfixes completely missed doing this before, but in case you still need it, as promised:
LMK if this answers your question(s) and if there's anything else I can help with.

handover brain dump

Context on stdout/stderr and how xcpretty works

In order to capture build errors in a tty, either one of these needs to happen:

  • The error was logged to stderr
  • The error was logged to stdout and not swallowed by the process that's
    reading that stream.

One of xcodebuild's problems is that it prints some failures to stderr,
and the rest to stdout. In our case the process that consumes stdout from
xcodebuild is xcpretty. This means that with using the current "whitelist"
approach, some errors ultimately get swallowed and users get confused why
certain builds fail. A common example of this are various code signing errors.

One way of mitigating this is using tee to dump the raw log into a separate
file, and pretty printing in tty with xcpretty. That's how people have
been solving it so far. The downside of the whitelist approach is that people
need to host raw logs somewhere, developers need to locate them after the
fact the build has failed, and some log files can have tens of megabytes.

Lastly, because of this approach, we can't record both stdout and stderr
in the raw log, so users end up seeing different content on their tty and
log files. This is why there's a blacklist branch in the repo, completely
changing the approach.

Blacklist vs Whitelist

In the whitelist approach, xcpretty takes input line by line and pretty
prints one or more lines in the context it knows about. Sometimes it's only
one line, sometimes it needs to enter a state and keep collecting lines in
order to generate a helpful error message. An example of this is a failing
CompileC command:

/path/to/file.m:47:12: error: returning 'float' from a function with incompatible result type 'NSNumber *'
    return (self.floatValue * 60);
           ^~~~~~~~~~~~~~~~~~~~~~

If xcpretty knows how to parse this failure, it'll parse and print it out.
Otherwise, it'll swallow the input and print a newline.

This is why it's called whitelisting: it prints only the lines it knows
about, and suppresses the rest. Blacklisting is the exact opposite: prints
everything to the user, and suppresses the lines that shouldn't be printed.
Blacklisting approach allows us to redirect stderr to stdout and let
xcpretty consume both through the same pipe.

The usage would look something like this:

xcodebuild 2>&1 | xcpretty
#          ^                ^
#          |                L No need for | tee xcodebuild.log
#          |
#          redirect stderr->stdout

As Apple's toolchain constantly evolves, some of the outputs keep changing.
xcpretty needs reflect those changes in order to correctly pretty-print
output to the users. With the whitelist approach, logs silently stop being
displayed. With the blacklist approach, the worst thing that could happen is
users seeing an ugly verbose log. This is arguably better than not seeing
anything.

Support default OSX ruby

The average person installing xcpretty is an iOS dev or a CI machine.
Neither of these two are seasoned Ruby developers, with Ruby version managers
and a bleeding edge Ruby installation. Keep this in mind when distributing
software - even with our simplistic approach people sometimes open issues
related to Ruby distributions and paths.

What worked well

Tests

The project was written in a TDD fashion, which led to a testable and simple
codebase, and allowed us to keep moving confidently without fear of breaking
existing functionality. Besides that, there are E2E tests written in
Cucumber. These tests could be useful to test a cross implementation of
xcpretty, e.g. in Swift. Besides these two, we also have a performance test
running as a part of the standard unit tests, and it ensures that xcpretty
runs faster than piping the same output to cat (it prints less content).

No dependencies

One thing that definitely eased the pain of distributing xcpretty was the
early decision to use no dependencies. Over time we added an optional
dependency on rouge (only for syntax coloring of snippets), but xcpretty
can run without any dependencies. Remember: xcpretty is primarily a CI
tool, and secondarily an ambitious iOS dev's terminal toy.

What didn't work so well (personal opinion)

xcpretty started as a build log formatter and a "do one thing, but do it
right" kind of tool. Because it was so easily extensible, people quickly
added features (like HTML reporting, json compilation database, plugins,
etc.) that became a maintenance burden. We didn't expect so many people would
be using xcpretty, and especially all those extra features. If someone is
continuing to maintain this project, perhaps look into splitting these
features out of the core tool.

Swift?

If I was writing something like xcpretty today from scratch, I'd probably
use a language the most of it's users will be familiar with. At the time
Delisa and I wrote xcpretty, Ruby was as hot as Objective-C in the iOS
community and widely used: tools like CocoaPods are written in Ruby,
Homebrew, Fastlane, you name it. The big downside of this approach is that
we immediately excluded perhaps the most important audience from potentially
submitting fixes.

@woodcockjosh
Copy link

can someone please publish a new gem that forks away from this repo so we can get some new development in please? Not a ruby developer but would really like to see some updates on xcpretty.

@ianfixes
Copy link
Author

Either that or add a new member to the organization who can start triaging all the PRs

@supermarin
Copy link
Contributor

@ianfixes Haven't heard back from you from the writeup I did above - are you still interested?
I'm not in the org anymore but can ping Felix or someone to add you.

@TheWirv
Copy link

TheWirv commented Oct 28, 2021

What's the current status on this @ianfixes?

@ianfixes
Copy link
Author

Based on employment, I'm unfortunately no longer in a position to take on a stewardship role in this project. But it does seem like the xcpretty organization on github needs to be expanded to include others who can pick this up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants