Torsten Bögershausen [off-list ref] writes:
On 2014-03-14 23.09, Junio C Hamano wrote:
quoted
* ap/remote-hg-skip-null-bookmarks (2014-01-02) 1 commit
- remote-hg: do not fail on invalid bookmarks
Reported to break tests ($gmane/240005)
Expecting a reroll.
I wonder what should happen here.
The change breaks all the tests in test-hg-hg-git.sh
(And the breakage may prevent us from detecting other breakages)
The ideal situation would be to have an extra test case for the problem
which we try to fix with this patch.
Antoine, is there any way to make your problem reproducable ?
And based on that, to make a patch which passes all test cases ?
After re-reading the thread briefly (there're just five messages)
http://thread.gmane.org/gmane.comp.version-control.git/239797/focus=240069
I think the "breakage" the patch tries to fix seems to be of dubious
nature in the first place ("I don't know how I ended-up with such a
bookmark", Antoine says in $gmane/239800), and it has been in
"Expecting a reroll" state in response to "I will try to come-up
with an improved version" in $gmane/240069 but nothing has happened
for a few months.
At this point I think it would be OK for me to discard the topic
(without prejudice); if the root cause of the issue (if there is
one) and a proper fix is discovered in the future, the topic can
come back with a fresh patch, but it appears to me that keeping the
above patch in my tree would not help anybody.
Thanks.
On 17.03.2014, at 18:01, Junio C Hamano [off-list ref] wrote:
Torsten Bögershausen [off-list ref] writes:
quoted
On 2014-03-14 23.09, Junio C Hamano wrote:
quoted
* ap/remote-hg-skip-null-bookmarks (2014-01-02) 1 commit
- remote-hg: do not fail on invalid bookmarks
Reported to break tests ($gmane/240005)
Expecting a reroll.
I wonder what should happen here.
The change breaks all the tests in test-hg-hg-git.sh
(And the breakage may prevent us from detecting other breakages)
The ideal situation would be to have an extra test case for the problem
which we try to fix with this patch.
Antoine, is there any way to make your problem reproducable ?
And based on that, to make a patch which passes all test cases ?
After re-reading the thread briefly (there're just five messages)
http://thread.gmane.org/gmane.comp.version-control.git/239797/focus=240069
For some reason, that link does not contain all messages from that conversation (unfortunately, I have seen GMane do that on multiple occasions. I hence try not to rely on it for reviewing email history -- I just don't trust it). In particular, it misses this crucial post:
http://thread.gmane.org/gmane.comp.version-control.git/239830
I call it crucial because it describes how to make a reproducible test cases out of this, in which a legitimate hg repository leads to a remote-hg error preventing the user from normal operation.
I think the "breakage" the patch tries to fix seems to be of dubious
nature in the first place ("I don't know how I ended-up with such a
bookmark", Antoine says in $gmane/239800), and it has been in
"Expecting a reroll" state in response to "I will try to come-up
with an improved version" in $gmane/240069 but nothing has happened
for a few months.
At this point I think it would be OK for me to discard the topic
(without prejudice); if the root cause of the issue (if there is
one) and a proper fix is discovered in the future, the topic can
come back with a fresh patch, but it appears to me that keeping the
above patch in my tree would not help anybody.
The (or at least "a") root cause has actually been discovered. Would a patch that adds an xfail test case for it be acceptable?
As to the why the proposed patch causes test failures: I think this is due to the fact that remote-hg inserts a fake "master" branch (resp. "bookmark" in the hg terminology). Now, in those test cases, a hg repository gets created that actually contains a "null" bookmark named "master". When the proposed fix for the problem is added, this bookmarks gets ignored. But at that point, remote-hg already determined that there is a hg bookmark named "master", and adjusts how it works accordingly -- when we then remove that bookmarks, things go awry.
But I might be wrong here, and in any case, did not yet have time to come up with a proper fix. What I do have is a test case, that I could turn into an xfail test. As a matter of fact, I a know a few more bugs in remote-hg for which I could produce xfail test cases. Of course I'd prefer to put them in together with a fix, but I don't know when I can get to that, if ever. So, would such changes be welcome?
On 19.03.2014, at 11:53, Max Horn [off-list ref] wrote:
On 17.03.2014, at 18:01, Junio C Hamano [off-list ref] wrote:
quoted
Torsten Bögershausen [off-list ref] writes:
quoted
On 2014-03-14 23.09, Junio C Hamano wrote:
quoted
* ap/remote-hg-skip-null-bookmarks (2014-01-02) 1 commit
- remote-hg: do not fail on invalid bookmarks
Reported to break tests ($gmane/240005)
Expecting a reroll.
I wonder what should happen here.
The change breaks all the tests in test-hg-hg-git.sh
(And the breakage may prevent us from detecting other breakages)
The ideal situation would be to have an extra test case for the problem
which we try to fix with this patch.
[...]
The (or at least "a") root cause has actually been discovered. Would a patch that adds an xfail test case for it be acceptable?
As to the why the proposed patch causes test failures: I think this is due to the fact that remote-hg inserts a fake "master" branch (resp. "bookmark" in the hg terminology). Now, in those test cases, a hg repository gets created that actually contains a "null" bookmark named "master". When the proposed fix for the problem is added, this bookmarks gets ignored. But at that point, remote-hg already determined that there is a hg bookmark named "master", and adjusts how it works accordingly -- when we then remove that bookmarks, things go awry.
But I might be wrong here, and in any case, did not yet have time to come up with a proper fix.
Actually, scratch that, I just came up with a fix, and also tests. Will submit shortly.
I'd still like to know whether it is OK to submit further patches with (x?)failing test cases?