On Sun, Nov 8, 2009 at 4:10 AM, Antoine Martin <[email protected]> wrote:
> Nathaniel Smith wrote:
>> 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)
Okay... but that patch just adds a try/catch around the initial client
socket creation, and only catches socket errors. It only affects the
initial connection, and even then it doesn't affect SIGINT handling...
Also, I'm running karmic and hitting control-C on the client doesn't
trigger apport for me...
>> 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.
Look again:
http://xpra.devloop.org.uk/patches/catch-socket-error.patch
You're thinking of something else...?
>> 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?
Well, it's pretty trivial anyway, but:
http://code.google.com/p/partiwm/source/diff?spec=svn9c713271d55dcb6fa5ad580b2e36af9b09d6389e&r=9c713271d55dcb6fa5ad580b2e36af9b09d6389e&format=side&path=/parti/parti_main.py
>> 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.
Making code readable for these IDEs requires making it less readable
for humans? That seems like quite a limitation :-).
Certainly they're handy, and I did apply various of the changes they
suggested, that I wouldn't have noticed otherwise. But they have false
positives, too; that's the nature of lint tools.
>> 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.
Yes, I noticed that. I originally designed 'wimpiggy.trap' to be used
with 'import *', and then broke that slightly when I added the logging
code, so I fixed 'wimpiggy.trap' by defining __all__. Maybe it would
be better to stop using 'import *', I dunno, probably doesn't make
much difference, but for now I just standardized on 'import *' because
it was the majority.
>> 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.
Ah, right, thanks for the reminder.
Okay, I just committed the version where it shows the actual machine
where the app is running by default :-). Do you still care about the
command line option, given that?
>> 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.
Yes, I did that a while ago, these remaining ones are the ones that
involve adding spaghetti if/then's for different platforms to the main
code... (AFAICT)
> 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.
Yes, of course. My point is just that these patches are not about
changing any core functionality (that part, e.g. your patch for the
glib mainloop's tricky behavior on win32, was already merged), they're
about stubbing out stuff that doesn't work on win32. I would like to
do that in a cleaner way (i.e., by separating out platform-specific
code into its own module, rather than having if/then's scattered
around the whole program), and these patches are very useful in
identifying which bits need to be separated out, but *applying* them
won't help much with that.
>> 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.
All I mean is that TCP is never going to be as secure as ssh, and in
most situations will be substantially less secure, and that very few
users have the technical knowledge to make an informed judgment
between them -- yet the option is supported and documented, so they
have to make such a judgment anyway, and some will get it wrong. (I've
talked to at least one person on IRC who AFAICT was using TCP because
it must be faster. This is almost certainly wrong, I'm not at all sure
it was secure in their situation, and they'd have been better served
if the option just didn't exist.)
Which parts of this are not backed up by facts?
> ("keep not getting an answer" - no comment, see archives)
You're right, you did give the reasons below the last time we talked
about this -- I'm sorry I forgot about that. Part of why I forgot the
details, though, is that I am still not very satisfied with these
reasons :-).
Also, I'm not sure you're answering *quite* the question I asked: I
didn't ask what the potential theoretical benefits of TCP support were
in general, I asked why *you* wanted it (since obviously you are
willing to put a lot of effort into writing patches and attempting to
overcome my pigheadedness :-)).
> 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!?
Why require support for raw TCP sockets when it's just not necessary? ;-)
Let's stop talking hypotheticals. When you, personally, want to log
into a remote machine over a secured network, do you use ssh or
telnet?
Do you, personally, insist that sysadmins install and support telnet
servers, because you refuse to use ssh over secure networks?
If the network is *really* secured, then why do you need password support?
If it isn't really secured, enough so that you do need password
support, then how do we know it is secure enough that you don't need
the many other security features ssh provides?
> * 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.
Right now, to me, this seems like the only real argument. But this is
so far outside xpra's core use case, and describes a situation that is
so likely to require special infrastructure *anyway*, that I don't
find it very convincing on its own.
> * Win32 and SSH don't get on. Putty/Plink is no go and openssh would
> require cygwin. Keep it simple.
AFAIK, I've made them "go" -- though I'm still waiting for someone to
confirm that, see the other thread. Is there something I'm missing?
-- Nathaniel
_______________________________________________
Parti-discuss mailing list
[email protected]
http://lists.partiwm.org/cgi-bin/mailman/listinfo/parti-discuss
