Nathaniel Smith wrote:
> On Tue, Oct 13, 2009 at 8:11 AM, Antoine Martin <[email protected]> wrote:
>> For those who still want to get a better xpra, here they are. Including
>> fixes, MS Windows support, a gui launcher, avoids triggering apport on
>> karmic, jpeg compression...:
>> http://xpra.devloop.org.uk/patches/
>
> I just did a quick pass through these:
>
> avoid-apport-whenkilled.patch: Fails to apply. What is it trying to do?
Ubuntu Karmic (and probably others) will catch a python unclean exit
(ctrl^C, SIGINT) and launch apport to report an application crash upstream.
In this case, it is not a crash, so it is best to catch the exception
and exit cleanly (with an error code if needed)
> bug-xerror-undef-var.patch: Applied.
>
> catch-socket-error.patch: ...this is the same as
> avoid-apport-whenkilled.patch?
Nope, this one is a real bug: the log statement uses a variable ("e")
which does not exist.
> cleanup-parti-main-unusedimport.patch: Fixed in a better way.
Can you elaborate?
What's better than removing one line of unused code? Removing two?
> cleanup-parti-scripts-unusedvars.patch: (options, args) =
> parser.parse_args(...) is idiomatic; this patch makes the code harder
> to read, not easier...
The aim was not to make it easier or harder to read, just to remove
unused variables (hence the name of the patch).
This allows IDEs (pydev/lint) to find real bugs automatically, they have
already found a few.
> cleanup-wimpiggy-prop-unusedimportsandvars.patch: Applied the part of
> this that I like.
> cleanup-wimpiggy-unusedimportsandvars.patch: Applied the parts of this
> that I like.
> cleanup-xpra-scripts-unusedimportsandvars.patch: Ditto.
> cleanup-xpra-unusedimportsandvars.patch: Ditto.
> cleanup-yatest-whitespace-import.txt: Ditto.
I hope this includes the changes to the import statements for
wimpiggy.error, as this otherwise imports the logger twice which is
unnecessary at best, potentially worse.
> client_launcher.py: This could be integrated as 'xpra gui-attach' or
> something, I suppose...?
I don't mind.
> missing-sys-import.patch: Already fixed in trunk.
OK.
> set-socket-timeout.patch: Applied.
Good.
> xvfb-bin.patch: Rewrote in a slightly way, and documented. It is now
> called "--xvfb", in line with the existing --ssh and --remote-xpra.
OK.
> xpra-name-windows.patch: Can you remind me what the point of this is?
So you can set the title of the xpra client window. You suggested
passing it from the server since it is a window attribute. I would be
happy with that, but until that's implemented/working this patch is
quite useful.
> jpeg-compression.patch: Deferring until I can think about it more...
> the UI seems annoying, in that really xpra should be smart enough to
> use jpeg only when it matters, not require you to fiddle with some
> magic knobs that most users won't discover or know how to tweak
> properly. (I'm not at all sure how to tweak them myself... does using
> jpeg even actually help in practice? How was this measured and what
> levels of compression worked best?)
This patch is not mine, see this thread which has some rough figures:
http://www.mail-archive.com/[email protected]/msg00266.html
(CCed author)
I think it would be more likely to help improve xpra if it was merged:
then it is just a question of adding the logic to switch the compression
protocol on the fly as needed rather than using "magic knobs". This is
far less likely to happen if the code stays out of tree.
> mswindows-cant-clipboard-asis.patch
> mswindows-conditional-pyrex.patch
> mswindows-disable-server-code-in-main.patch
> mswindows-exec-main.patch
> mswindows-ignore-sendclientmessage.patch
> mswindows-keyboard-not-impl.patch
> mswindows-move-common-parts.patch
> -- Continuing to defer these until there's a chance to get it right.
You could at least merge the parts which are uncontroversial and make
the code better.
Also "getting it right" will probably have to rely on these patches in
some shape or form: generic win32 does not and will not support X11 (I'm
not talking about Cygwin etc), so the clipboard simply cannot be
implemented via the current ClipboardProtocolHelper for example.
> xpra-allow-bind-to-all-interfaces.patch
> password-file.patch
> password-secure.patch
> password.patch
> -- Okay, seriously: Why do you care about TCP? Is it just because
> implementing ssh support on windows is hard? Because I keep asking
> this, and I keep not getting an answer, just more and more patches to
> enhance an option that is fundamentally insecure and misleads users
> into doing insecure things. Please?
Seriously, please refrain from claiming things like "fundamentally
insecure and misleads users", as it's just not backed up by facts.
("keep not getting an answer" - no comment, see archives)
Some of the benefits of plain TCP are (you can google the details and
the ones I have mentioned before):
* SSH is not necessary on a secured network. Why require things that
just aren't necessary!?
* SSH is simply not desirable in some use cases: if I want to give
access to a session, having to hand out SSH accounts (keys, passwords
whatever) for just binding to a port is crazy.
* Win32 and SSH don't get on. Putty/Plink is no go and openssh would
require cygwin. Keep it simple.
Cheers
Antoine
_______________________________________________
Parti-discuss mailing list
[email protected]
http://lists.partiwm.org/cgi-bin/mailman/listinfo/parti-discuss
