| mgz | morning! | 07:54 |
|---|---|---|
| jelmer | hi mgz | 08:04 |
| jam | mgz: morning! | 08:15 |
| mgz | hey jam | 08:19 |
| mgz | we gatecrashing red's call this morning? | 08:19 |
| mgz | and now I see we are... | 08:20 |
| === mmrazik is now known as mmrazik|lunch | ||
| === mmrazik|lunch is now known as mmrazik | ||
| jam | mgz: so what can I do for you to get: https://code.launchpad.net/~jameinel/bzr/2.5-unending-sendall-1047309/+merge/123268 merged? | 11:17 |
| mgz | jam: maybe we should just merge it, but I don't really like heuristic workarounds for upstream bugs | 11:24 |
| jam | jml: I think I have a fix for your issue that has landed in lp:bzr/2.5, are you comfortable testing it out and confirming? | 11:25 |
| jam | mgz: I don't see an explicit statement that 'send must not return 0 bytes'. | 11:25 |
| jam | so I can't say it is strictly a bug in paramiko. | 11:25 |
| jam | and we still need to workaround it in bzr for whatever versions people are going to use/we are going to ship. | 11:25 |
| jml | jam: sure. | 11:25 |
| jam | mgz: At this point I'm much more on the "lets get it working" side of the fence. | 11:26 |
| jam | jml: It hasn't landed in trunk yet, because I have a couple other fixes I want to land, and just merge up 1 tiem. | 11:26 |
| mgz | well, in this case returning 0 means ECONNRESET... but we don't know if that's what it'll always mean | 11:26 |
| mgz | I think I'd prefer treating it as always ECONNRESET and muttering something, were it not for the fact this isn't a well exercised codepath in general usage | 11:28 |
| jml | jam: I can run 'bzr di' successfully in the lightweight checkouts I have using lp:bzr/2.5. | 11:33 |
| jam | mgz: well it arguably means ssh.EOF | 11:34 |
| jam | or EPIPE | 11:34 |
| jam | or... | 11:34 |
| jam | (paramiko does have a way to single that it is disconnected, but I'm not getting that here, and I really don't know why.) | 11:34 |
| vila | jam, mgz: http://developerweb.net/viewtopic.php?id=2956 | 12:37 |
| vila | jam, mgz : From that, I'd say: send() returns 0 because it expects something else to call recv and get the EPIPE, a test should be able to reproduce and then the unknown can vanish | 12:39 |
| jam | vila: send returns 0 because of paramiko | 12:39 |
| jam | completely orthogonal to EPIPE | 12:39 |
| vila | jam: and why does paramiko do that ? | 12:39 |
| jam | only particularly related because paramiko looks like a socket to our code | 12:39 |
| vila | I fail to see how that answer my question | 12:41 |
| jam | vila: it doesn't, it finishes my sentence. | 12:45 |
| jam | paramiko does X, socketpair does Y | 12:45 |
| jam | socketpair is raising a EPIPE | 12:45 |
| jam | paramiko is returning 0 bytes sent | 12:46 |
| jam | windows was using an actual PIPE to the subprocess, and was getting something else. | 12:46 |
| jam | sometimes EINVAL, sometimes EPIPE | 12:46 |
| vila | jam: forget about windows when it comes to sockets, that's not the best implementation to understand what *can* happen, I won't call it buggy but it certainly only exhibit only a subset of the possible behaviors | 12:47 |
| jam | (we use a socketpair to communicate to the subprocess when we can, because then we can recv(1024) without worrying about blocking, rather than having to read(1) when we don't know how big the next reques tis) | 12:47 |
| vila | the point of the url above is that the EPIPE *may* be available on the read side and the write side is then having a weird behavior of returning 0 on send | 12:48 |
| jam | vila: *paramiko* is returning 0 on send, which is not a socket | 12:48 |
| jam | we are getting an EPIPE on the write side | 12:49 |
| jam | (not necessarily SIGPIPE, which is what you linked) | 12:49 |
| vila | jam: what matters is that paramiko *is* using sockets, no matter what it presents to you | 12:49 |
| jam | vila: you're missing what actual errors we are getting running things in the world. | 12:49 |
| vila | ? | 12:49 |
| jam | vila: If we use BZR_SSH=paramiko, and get disconnected, we hang indefinitely because paramiko returns 0 bytes for the send, and no error. | 12:50 |
| jam | so I translate that into some other error. | 12:50 |
| jam | If we use BZR_SSH=openssh on Linux, we communicate with the 'ssh' subprocess using socketpapi | 12:50 |
| jam | socketpair | 12:50 |
| jam | and we get socket.error(EPIPE) raised when we try to send() to it. | 12:50 |
| vila | So what ? You're not explaining why paramiko behaves differently | 12:51 |
| jam | vila: because it does | 12:51 |
| jam | You're welcome to dig into the paramiko code to explain why it doesn't behave identically to a socket to us. | 12:51 |
| vila | saying X does Y doesn't explain why X does Y | 12:51 |
| jam | I don't really care enough. | 12:51 |
| jam | it is really hard to maintain paramiko, so I'd rather just work around it. | 12:52 |
| jam | given all versions across all platforms that bzr-2.5 should be available on. | 12:52 |
| vila | fair enough, I'm just saying that working around without understanding is not the best way to avoid bugs in the long term | 12:53 |
| vila | and one the key points here is the difference between shutdown and close, but hey, it's up to you to dig that or not | 12:53 |
| * jelmer totally digs those socket shenanigans | 12:54 | |
| jam | vila: given that it is being done in your free time, you are welcome to devote as much time as you would like digging into the paramiko case :) | 12:58 |
| vila | the more cruft added there the less incentive it is to me to touch this code :) | 12:59 |
| === mmrazik is now known as mmrazik|afk | ||
| ahasenack | I have a branch at r656 which gets merge conflicts when I merge it with another one | 13:42 |
| ahasenack | at r652, the merge works fine | 13:42 |
| ahasenack | now, my question is about bzr revert | 13:43 |
| ahasenack | if I do "bzr revert -r 652" and commit, and then merge, there are the conflicts | 13:43 |
| ahasenack | but, if I do "bzr branch -r 652" into a new branch, and merge that one, there are no conflicts | 13:43 |
| ahasenack | shouldn't the result of "bzr revert -r 652 + commit" and "bzr branch -r 652" be the same? | 13:43 |
| ahasenack | I thought revert means "bring the branch back to that revision" | 13:44 |
| ahasenack | and branch is "copy out that revision into a new branch" | 13:44 |
| ahasenack | funny, there is no difference in the code between the result of "bzr revert -r 652" and "bzr branch -r 652" | 13:52 |
| ahasenack | yet only one of them merges without conflicts | 13:52 |
| ahasenack | is this expected or a bug in bzr? | 13:53 |
| jelmer | ahasenack: it does have an effect on the history | 13:55 |
| jelmer | even f the contents are the same | 13:55 |
| ahasenack | I wanted history record that I had to revert to 652, otherwise I would have used uncommit | 13:58 |
| ahasenack | would have bzr merge . -r 653..652 (for example) produced a more merge-friendly result than revert -r 652? | 13:59 |
| jelmer | ahasenack: I think that would have the same result as the revert | 14:01 |
| ahasenack | so reverts are bad for merges? uncommits are better, if possible? | 14:02 |
| ahasenack | a branch is merged, but introduces a bug | 14:03 |
| ahasenack | you quickly revert | 14:03 |
| ahasenack | the bug is fixed, and you merge again | 14:03 |
| ahasenack | that's my use case, except now it's conflicting | 14:03 |
| ahasenack | hm, revert leaves things a bit nutty | 14:14 |
| ahasenack | after bzr merge, bzr status tells me it thinks it needs to add a .OTHER file | 14:15 |
| ahasenack | Contents conflict in canonical/landscape/schema/main/patch_147.py | 14:17 |
| ahasenack | and then | 14:17 |
| ahasenack | $ bzr st | 14:17 |
| ahasenack | added: | 14:17 |
| ahasenack | canonical/landscape/schema/main/patch_147.py.OTHER | 14:17 |
| ahasenack | :) | 14:17 |
| ahasenack | ok, let me rephrase that as :( | 14:31 |
| jpds | ahasenack: bzr remove canonical/landscape/schema/main/patch_147.py.OTHER ? | 14:40 |
| ahasenack | jpds: it only showed its head after bzr merge, it was never "bzr add"ed to the tree, I can't explain that | 14:40 |
| ahasenack | I had to revert the revert, then merge | 14:43 |
| === mmrazik is now known as mmrazik|afk | ||
| vila | ahasenack: if I'm not mistaken, the OTHER branch added patch_147.py and so did the THIS branch, hence the conflit. | 16:00 |
| ahasenack | vila: it was weird indeed, that patch introduced a bug | 16:00 |
| ahasenack | vila: so I reverted it, while it was fixed upstream | 16:00 |
| ahasenack | vila: then I merged again, and there was the conflict | 16:00 |
| vila | ahasenack: if you dig the history with (bzr st -rx..y --show-ids) you can probably confirmed that and find some revision were the file is removed/added | 16:00 |
| ahasenack | vila: I had to undo the revert, then merge | 16:00 |
| ahasenack | also, it thought the code I reverted was there. Without the undo part, the merge wouldn't bring in the fixed code, it was very confusing | 16:01 |
| ahasenack | lesson: be careful with bzr revert | 16:01 |
| vila | ahasenack: right, so probably reverting put you back in a time where the file had a different file-id | 16:01 |
| ahasenack | it was like 652: ok; 653: introduced the bug; 654: revert -r 652 | 16:02 |
| ahasenack | and then things got out of hand | 16:02 |
| ahasenack | I had to undo 654 | 16:03 |
| vila | ahasenack: it's not 'revert' per-se, more an issue with a file removed/added in one branch | 16:03 |
| ahasenack | that patch file was added in 653, and "removed" in 654 via bzr revert | 16:03 |
| vila | ahasenack: bzr st --show-ids -r652..653 ? | 16:03 |
| vila | ha right | 16:03 |
| ahasenack | yeah | 16:03 |
| vila | *that* is the "bug" | 16:04 |
| vila | you fooled bzr :-/ | 16:04 |
| ahasenack | something I should file? | 16:04 |
| vila | (which is a bzr bug in itself to be fool-able this way) | 16:04 |
| === deryck is now known as deryck[lunch] | ||
| ahasenack | my situation is fixed not, btw, after I did that undo, then the merge worked fine | 16:05 |
| ahasenack | s/not/now/ | 16:05 |
| vila | unfortunately, I don't think so, it's the same underlying issue than "parallel import" | 16:05 |
| * vila nods | 16:05 | |
| vila | ahasenack: and you probably won't fall in this trap again | 16:06 |
| ahasenack | oh, who knows :) | 16:06 |
| ahasenack | I'm not sure there was another way out of this | 16:06 |
| ahasenack | unless I used uncommit | 16:06 |
| ahasenack | once I detected the bug | 16:06 |
| ahasenack | but I wanted to preserve history, I wanted it to be recorded that a bug was found and the code had to be reverted | 16:06 |
| ahasenack | and I did use revert in other occasions | 16:07 |
| vila | ahasenack: a reverse merge is the working way | 16:07 |
| ahasenack | but those probably didn't have an added file | 16:07 |
| ahasenack | vila: bzr merge . -r N+1..N? | 16:07 |
| vila | yup | 16:07 |
| ahasenack | ok | 16:07 |
| vila | it keeps track of the file-ods | 16:07 |
| vila | ids | 16:07 |
| ahasenack | good to know, and I find it more intuitive too | 16:08 |
| vila | the root issue here is adding the same file (at the same path) with a *different* file-id which is what the confusing 'content conflict' is about | 16:08 |
| ahasenack | a reverse patch | 16:08 |
| vila | content here refer to the inventory content: two files want to occupy the same path (they are different as far as bzr is concerned because they have different file-ids) | 16:09 |
| ahasenack | the content is different indeed | 16:09 |
| ahasenack | but something feels off with revert | 16:09 |
| ahasenack | for example, the 653 changeset brought in other fixes, not just the buggy patch_147.py one (new file) | 16:10 |
| ahasenack | after I reverted that | 16:10 |
| vila | but for mere mortals ignoring file-ids, it's (as you put it) nutty | 16:10 |
| ahasenack | and then merged again with upstream after patch_147 was fixed, the other fixes I reverted weren't brought in again | 16:10 |
| ahasenack | it's as if bzr considered them applied | 16:10 |
| ahasenack | it was pretty "nutty", I ran bzr merge, got those conflicts, and a plain diff between the two branches still showed many changes that were not brought in | 16:11 |
| ahasenack | all part of that reverted 653 commit | 16:11 |
| ahasenack | anyway, lunch time | 16:13 |
| ahasenack | vila: thanks for the discussion | 16:13 |
| vila | ahasenack: not sure how to help here without seeing more details (not sure it's worth the trouble if you're now safe) | 16:13 |
| vila | ahasenack: my pleasure | 16:13 |
| === deryck[lunch] is now known as deryck | ||
| === yofel_ is now known as yofel | ||
| === spm is now known as stevemci | ||
| === stevemci is now known as spm | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!