bcpierce00 / unison

Unison file synchronizer
GNU General Public License v3.0
4.08k stars 229 forks source link

🐛(WinOS) `unison` (gui/gtk) has lost all console output (between v2.51.4 and v2.51.5) #778

Closed rivy closed 2 years ago

rivy commented 2 years ago

The WinOS GUI version of unison no longer outputs anything to the console. Unfortunately, this makes using -ui text, -help, -version essentially unusable, and no errors are ever displayed when incorrect argument/option text is supplied.

>"unison-v2.51.4+ocaml-4.12.0+x86_64.windows\bin\unison-gtk2.exe" -version
unison version 2.51.4 (ocaml 4.12.0)

>"unison-v2.51.5+ocaml-4.12.0+x86_64.windows\bin\unison-gtk2.exe" -version

>"unison-712599e7+ocaml-4.14.0+x86_64.windows\bin\unison-gui.exe" -version

In looking through the difference (https://github.com/bcpierce00/unison/compare/v2.51.4...v2.51.5), I suspect the problem is in the new console handling code which is likely not piping output back to the original console.

rivy commented 2 years ago

@gdt

The large amount of unfamiliar code in that diff made me initially overlook b423d1d37b43ee97; that's the culprit.

Reverting that commit restores console output. I've pushed a build run with that reversion (4e5044a4373) on top of 'master' and it now seems to be functioning normally from the command line (see https://github.com/rivy/unison/actions/runs/2892772573 for the relevant build artifacts).

tleedjarv commented 2 years ago

This was an intentional change (Windows-specific issue and Windows-only change).

unison-gui.exe is not intended to be used interactively on the command line. Use unison.exe for that.

Is there a specific reason you want to get text output from a Windows GUI application? Is that even possible?

rivy commented 2 years ago

Incorrect.

The unison-gtk2 executable has always had dual-use console and GUI functionality, hence the -ui [text | graphic] option. It has been very usable in either mode depending on that -ui flag, unison-gtk2 -ui text has been essentially equivalent to text-mode only unison for all the years that I've used and contributed to it. You can use it in text mode most of the time and then just add -ui graphic to a given command line to flip on the UI when something becomes more complicated. It works just the same as the POSIX GUI client, text or GUI as you wish it. I think it's quite elegant.

Is there some specific problem that was solved by that change? There was no referenced issue about any problems. And the change severely breaks the UI/UX of the GUI client. As I noted above. -version and -help show no output; and starting it with incorrect options gives nothing back to the user. The application just fails to appear.

tleedjarv commented 2 years ago

All this is still true for non-Windows platforms where there are no changes.

The ui arg brings some convenience by allowing to distribute Unison as a single binary. Otherwise, as you also point out, you can always replace unison-gui -ui text with unison and unison -ui graphic with unison-gui.

There is no such convenience in Windows because you can't distribute the GUI as a single binary anyway. You always need a ton of DLLs for GTK. So you always have two binaries and if you need text or GUI you just change the exe name instead of adding or removing -ui text.

Please follow the GH PR reference from the commit for further information and motivation.

the change severely breaks the UI/UX of the GUI client

I think this change is a major quality of life improvement for Windows users. There are a few ways to address the shortcomings you highlight.

Some of the text output (like -version and cmdline parsing errors) from the GUI application could be captured and displayed in the GUI. An old-style text+GUI binary could be built as a third binary, resulting in unison.exe (or unison-text.exe), unison-gui.exe and unison-console+gui.exe, for example.

I don't think it's possible to create a Windows GUI application that can use the console if started from console, yet this would seem the solution you are looking for. If someone knows how to do it, I am more than happy to implement it.

gdt commented 2 years ago

I don't see the "can't have a single binary" as very relevant. While you can have a single static binary in unix, I view that as irregular instead of using shlibs -- but it's beside the point.

Is the point that it's difficult/kludgy to have a windows program that after parsing args decides to use the terminal like a normal unix program would, vs being a graphics-only program?

Separately from this, we have a rototill issue #667 on the table. I guess though that the question "should unison-gui -ui text work?" remains even if we do that.

rivy commented 2 years ago

All this is still true for non-Windows platforms where there are no changes.

👍🏻 For context, I'm "primarily" a WinOS user, but I use the Linux/POSIX version on just about all VMs, distant hosts, and appliances. And I'm not sure why Windows should differ here.

Please follow the GH PR reference from the commit for further information and motivation.

the change severely breaks the UI/UX of the GUI client

I think this change is a major quality of life improvement for Windows users. There are a few ways to address the shortcomings you highlight.

Could you point the PR/discussion out? I started from the commit itself, and there's no obvious reference to any discussion. I'd like to know what the issue of concern/quality of life improvement was...

I've always used this utility as a command line utility, adding -ui graphic when I've needed it for complex synchronizations. The referred change just seems partially broken and impacts my use negatively without any obvious gain.

Some of the text output (like -version and cmdline parsing errors) from the GUI application could be captured and displayed in the GUI. An old-style text+GUI binary could be built as a third binary, resulting in unison.exe (or unison-text.exe), unison-gui.exe and unison-console+gui.exe, for example.

Those suggestions are both reasonable. I would suggest that b423d1d37b43ee978d20a7f71458fc2b9d2d192 should be reverted until, at least, that GUI work has been done, so that users aren't left in the current situation of empty feedback when executed with various arguments/options.

I don't think it's possible to create a Windows GUI application that can use the console if started from console, yet this would seem the solution you are looking for. If someone knows how to do it, I am more than happy to implement it.

I feel like we have a disconnect here; I really don't understand this statement. I'm currently using it in the dual-use mode. I run the unison-gui executable from the command line, primarily as the text version using the -ui text option, and then add -ui graphic which pops up the GUI when helpful. And this use is exactly mirrored on all of my WinOS and POSIX hosts.

Ultimately, here, after being comfortably rooted to an earlier version for quite a while, I was testing out a version upgrade to the newest version (with the more flexible version inter-operation). This was a surprise, running into a partially non-functional artifact with surprisingly empty user feedback. I can definitely modify my usage pattern with aliases and wrappers to work around this issue, but I don't see good reason to decrease the functional correlation between the various platform executables. One of the huge strengths of unison has been the truly excellent cross-platform support as well as the ability to shift quickly between simple, straight-forward text usage and the GUI for more complex situations. I would hope that all executables continue operate in the most similar manner possible to allow essentially identical usage, no matter the underlying platform.

I am happy to lend some support to work out issues, with the caveat that my experience with OCaml language itself is almost negligible and my day job still has a lot of pandemic-associated variability.

tleedjarv commented 2 years ago

The PR in question is #579 (if you view the commit in GH web then you can easily navigate to the PR that included it).

The issue here is that in Windows there is a difference between console applications and (pure) GUI applications. The old binary was a console application that opened a graphical window. It works cleanly for you because you start the application from command line and therefore don't notice any difference. For users click-starting the (supposedly) GUI application, they suddenly get an annoying and (mostly) useless console window which they can't close. I think this is the disconnect you're referring to.

Is the point that it's difficult/kludgy to have a windows program that after parsing args decides to use the terminal like a normal unix program would, vs being a graphics-only program?

Yes, this is exactly the point. I don't know too much about Windows but my understanding is that in Windows an application can be either a console application or a GUI application and this difference is apparent already at startup before any user code is executed. A console application will always have a (visible) console window (opened automatically by Windows if not started from command line, otherwise reusing whatever console the command line has). A GUI application can still have a console window (and the PR includes this to output debugging, for example) but it always has to open a new one. This means that if a GUI application is started from command line, it cannot reuse the same console. (A note about terminology: I'm using the word "console" because all of this is strictly specific to Windows.)

I definitely do not recommend reverting the commit because this issue appears only for a specific niche usage. For vast majority of users (click-starting or script-starting the GUI, command line-starting or script-starting the TUI) the new way is an improvement or they're unimpacted. For some users (command line-starting the GUI) and only in very specific cases (listed in the first comment https://github.com/bcpierce00/unison/issues/778#issue-1344953718) this may bring unexpected results.

I don't see good reason to decrease the functional correlation between the various platform executables

I think we all agree with this sentiment. But the reality is that there is already a lot of Windows vs POSIX differences that have to be overcome (and many times not) in the code. Sometimes it is just not worth it. I would guess that most users prefer not having that useless dummy console window around, just to use a GUI application. If there is a solution in Windows where the same binary can smoothly be a clean GUI and also use the console when needed, I'd be all for it.

@rivy do you build your binaries yourself? If so then maybe the easiest solution is to make the change in this commit user-configurable at build time. But then again, I'd be really surprised if many users would be bitten by this.

acolomb commented 2 years ago

I'd like to add that having a dysfunctional command line option for unison-gtk.exe -ui text is nonsense though with the current state of things. There is no way to even notice what it does when launched like that and it is certainly not following the principle of least surprise.

I guess we should either have one binary capable of switching or two binaries where the GUI one has a sole purpose. If it's one binary, the Windows issue needs to be solved and then I'd probably not give it a special name. You either have Gtk support compiled in or not, but building a separate text UI binary when there is already a GUI one that covers text as well with an option seems silly.

How do other Windows GUI apps go about showing command line parsing errors? Seems like a common problem to solve.

A quick Google search for "windows output error to console from gui executable" for example points to this:

https://stackoverflow.com/a/32139037

tleedjarv commented 2 years ago

I'd like to add that having a dysfunctional command line option for unison-gtk.exe -ui text is nonsense though with the current state of things. There is no way to even notice what it does when launched like that and it is certainly not following the principle of least surprise.

I disagree. This is a very niche usage and only for expert users. Maybe I am completely misguided here but I would be really surprised if many users ever attempt this in Windows. I would see no problem in just stating that -ui text is not supported in Windows, period. I think -ui text on all platforms is an expert thing for very specific situations, never for regular users.

I guess we should either have one binary capable of switching or two binaries where the GUI one has a sole purpose. If it's one binary, the Windows issue needs to be solved and then I'd probably not give it a special name. You either have Gtk support compiled in or not, but building a separate text UI binary when there is already a GUI one that covers text as well with an option seems silly.

Having the same binary provide both TUI and GUI is a nice trick. But in reality this can't be the sole binary because this binary depends on GUI libs (X and GTK so-s or DLLs) and is therefore unusable on many systems. On those systems you need the text-only binary. Even if you have a system with all the required GUI support, having the same binary provide both GUI and TUI seems rather unconventional (again, maybe I'm completely misguided here) and is impractical. Even in Windows, you can copy and run unison.exe (the TUI binary) anywhere you want. unison-gui.exe on the other hand is tied to GTK DLLs that must be dragged around with it. I don't think a single binary is a good idea.

How do other Windows GUI apps go about showing command line parsing errors? Seems like a common problem to solve.

I don't know but why would this be common? On the contrary, I think the common solution is to not do this. If it's a GUI application then all the output is in GUI. A command line parsing error would probably be displayed as a GUI message box.

These Windows-specific questions are unfamiliar territory for me, so take everything I say with a grain of salt.

Edit: In summary: I do think this issue needs to be fixed. I just don't think it's a blocker. It's a minor issue. Further, I think this entire feature is unconventional and not that useful (even though I've myself used it on occasion) and I think one valid solution is to just drop the feature on all platforms (or only in Windows).

acolomb commented 2 years ago

Maybe I am completely misguided here but I would be really surprised if many users ever attempt this in Windows.

Totally right, I assume nobody does. Even more suspicious that there is a -ui text option in the binary at all. It does make sense on POSIX, where there is always a stdout / stderr stream even for GUI apps, although it's sometimes hard to figure out where it ends up.

I would see no problem in just stating that -ui text is not supported in Windows, period.

Just where to state that? On the homepage? In the manual? Best would be to print it to the console if invoked like that. Which we cannot use from the GUI binary... So yes, we could redirect all console output to GUI windows, that might help.

I don't know but why would this be common? On the contrary, I think the common solution is to not do this.

LibreOffice for example does the soffice.com / soffice.exe trick, but in fact when starting soffice.exe --help from PowerShell, it actually opens a new terminal window to show the output, waiting for a key press to close. soffice.com uses the invocation terminal. For cross-platform apps, I think it's common to have a regular CLI at least for options help, version output and to show invocation errors, even if the program is actually a GUI. Only Windows makes this hard, so projects which do care enough apparently build workarounds.

I think we actually agree on the fact that the dual-use GUI binary is an unconventional and cumbersome solution that should not be offered. I just wanted to explore the options as obviously some people have come to expect unison to work that way. My preferred solution as well would be to build totally independent binaries, each for a single UI style. That's also what the UISTYLE=... build option suggests, it doesn't say UISTYLE=text+gtk, but only one, mutually exclusive.

You're right it's unlikely to be using a full-blown Gtk stack if one just needs the text UI. But on the other hand, that is a builder / packager's decision to make and in theory one could distribute a single, statically compiled binary with text and Gtk UI included in one file. Probably nobody does that though, because it would waste a lot of space and in case of dynamic linking, pull in a load of unneeded dependencies if the user is only interested in the text UI. Thus I don't see a compelling need to have that "nice trick" of runtime UI selection at all. Just make two binaries, period.

Handling very basic output from that GUI executable even on Windows is still a soft requirement though, which this issue is about. The LibreOffice / InkScape model could be a good inspiration. But we should not aim for a complete solution to make unison-gtk.exe -ui text work again on Windows, rather make it fail with a warning to the user. Removing the -ui option altogether should be tracked in a new issue IMHO.

gdt commented 2 years ago

So maybe the way forward is:

acolomb commented 2 years ago

I wouldn't special-case Windows when dropping the -ui option. As explained above, I don't think it makes much sense anywhere, except for being a neat specialty.

Forcing errors to a GUI window might not catch all cases, such as when Gtk fails to initialize. For better unity among platforms, printing errors to the console should be preferred. On Windows, opening a terminal window for that case only when needed seems like an acceptable compromise, if redirecting to the invocation console is hard to implement. Aren't we doing something like that already for the case where SSH needs a terminal window on demand?

Agree that #667 would be a good related move. It will simplify the build system and better align with common practice.

gdt commented 2 years ago

I'm fine with dropping the -ui option for non-Windows after #667 is done. Right now, the build system is awkward, and a reasonable response by packagers it to package the gui and let people -ui text who want that, rather than doing two builds. Because it's the fault of the unison build system, not of packagers, as I see it, I don't want to impose churn on packagers unless there's a significant gain, and dropping -ui text isn't.

rivy commented 2 years ago

How about this as a compromise (PR #779)?

If/when unison is split into two pure applications (console-only and GUI-only) and the build process is refactored, the hybrid can be removed.

Would this be acceptable to everyone?

rivy commented 2 years ago

Alternatively, I've created a repository and built the simple console-gui-wrapper application used by Inkscape (mentioned by @acolomb; excellent find btw!).

It can work as a simple drop in sibling for any pure WinOS GUI application. When run from the command line, it launches the actual GUI and connects all of the standard IO streams without any messy extra windows.

Placing it into the WinOS bin directory as "unison-gui.com" means that when run from the CLI as unison-gui, the GUI functions is a POSIX-like manner with normal standard IO available, but without an extra windows being spawned.

The code is GPL licensed.

tleedjarv commented 2 years ago

I haven't looked at the console-gui-wrapper application closely yet. Do I understand correctly that it is completely universal code and not related to the GUI application in any way (besides the .com name)?

It still sounds complicated to use. Now there is a separate .com binary which depends on the user not specifying the extension on the command line. I guess it's better than nothing.

Meanwhile, I think your PR is a good start and I will soon also look into two additional options: presenting (some) text output in a GUI message box; and opening separate console window and pausing at the end for text output.

Note that the code to open the separate console window is already merged together with the commit referred above. This separate console is used for debug output (and for ssh on older Win which don't support pty). This same trick can be used to output --help and command line parse errors, we just need to pause at the end.

acolomb commented 2 years ago

If/when unison is split into two pure applications (console-only and GUI-only) and the build process is refactored, the hybrid can be removed.

Would this be acceptable to everyone?

Yes, at least it solves the immediate problem in this issue and doesn't block any further steps AFAICS.

You @rivy do write a lot about "the WinOS package". Is the project's attitude of providing only source code changing, or is it still just a tool for CI testing of the build process?

rivy commented 2 years ago

I'm not a maintainer, just an interested contributor, so I don't speak for the unison project.

TLDR: IMO, the project should supply a basic set of packaged executables, especially for POSIX-ish appliance and WinOS users.


I'm a long-time, frequent user of unison, on (and between) both POSIX and WinOS platforms. I struggled quite a bit to find compatible executables between POSIX (both normal distros and appliances [eg, Synology and QNAP NAS]) and WinOS. Until the recent (on-going) effort to create some standard wire protocol, all executables had to match unison version and OCaml version to interoperate.

The WinOS build, especially the GUI, is very difficult to build, requiring a lot of very specific prerequisite installations, configuration, and after-build package massage. So, after a review of available info and quite a lot of experimentation, I developed and then donated a GitHub workflow to the project that could build, test, and package compatible executables for multiple platforms, both POSIX (including limited appliances) and WinOS.

The workflow serves as a proven and very-specific recipe for the various platform builds. But following that recipe, especially the WinOS build, still requires a lot of resource and effort. For the various *nix distros, the package maintainers can produce these executables as a matter of course. But there's no group that produces executables/packages for POSIX appliances or WinOS.

But, I incorporated support for automated distribution into the workflow design... Whenever a version tag is pushed to the repository and successfully completes the builds and tests, all of the various completed packages are gathered and stored as associated assets within GitHub Releases for the project. For OSS projects like unison, GitHub stores and provides permanent public access to these artifacts, gratis. These packaged artifacts can be easily downloaded from the Releases section and make unison much more accessible to the wide world beyond just Linux distros.

So, since it's automatic and GitHub supplies the resources and storage (thank you, GitHub!), I think it's an easy decision to say, "yes, the project should supply packaged executables (at least static linux and WinOS packages)".

gdt commented 2 years ago

The project as a matter of doctrine considers what's checked in as the product and I don't see that changing.

I think it's really great that you did work on CI, and it's helping a lot of people. I rely on it, and the test extensions @tleedjarv did when deciding to merge. When things are actually wrong in CI artifacts -- as opposed to them not working in some environment -- I think we have a pretty good track record of addressing the underlying bugs.

The real underlying problem here is that there are a tiny number of volunteers, and the Free Software world has changed over the years where significant numbers of people expect what you'd get with paid support, even though they haven't paid for a support contract (and no the project doesn't do that). I personally have zero interest in dealing with Windows.

And yes, building on Windows is difficult. That's a bug in Windows, and in Windows software culture. I don't understand why there isn't a Free Software packaging system for Windows (native) that works well enough that we can just point to it. Cygwin and cross is pretty good, but there have been recent meltdowns because of buggy dependencies (#744).

I don't know what "POSIX applicances is". I know you know this, but for others: POSIX is a source-level interface specification, and so there is no such thing as a "POSIX binary". And, the world is more than Windows and GNU/Linux. Github CI is problematic in that it supports a tiny number of systems (#445).

Also, #705 is on the table. I view depending on proprietary github featuers as technical debt.

This is off topic for this bug; I ask everyone to continue, if they wish, in one of the mailnglists.

rivy commented 2 years ago

Yes, differing opinions, all from smart folks with sound reasoning. 😃

Back to topic, I'm working with @tleedjarv on the PR (#779); any concerns/blockers?