Stayed home from work yesterday, giving myself the excuse that I was sick. Probably should have gone to work instead, because it turns out I was feeling ok enough to do a bit of darcs-hacking. Also played hooky a bit this morning (it's 11:06), but overall, I'm glad to have spent time getting this stuff out of the way... clears my head, let's me focus on other things.
windows and weird filenames
A user on IRC was trying to work around
issue53 where Windows enforces a bunch of naming conventions like "don't start your filenames with AUX". I don't think I personally am going to do something about this, but my proposal is that, when we are running on Windows, we should treat patches that affect such files as affecting some other file instead, one with a translated name. So if we have a patch that affects AUX.dtd, we should apply the changes to -AUX.dtd or something; and print out a warning saying what we have done and why. This solution at least allows the patches to apply, but it does have some problems of its own, like (1) what if a file with the translated name already exists? (2) what if the user tries to modify the translated file? Do we want to recognise that? [I'd say, we probably should, but rather than let the user record changes, we just say "Whoops! you don't want to touch this file. Please mv it to something safe instead."] Mmmhh. I dunno if this is going to work, or if it's just going to make things super-complicated and introduce a bunch of random bloat. Well, in any case, we should at least
detect the problem and tell the user about it. I suppose simply ignoring such files (and printing a warning) is just as good, and much simpler to boot. (If we were to ignore them, we should at least try to handle the mv, though, although that too is complicated)
get --to-match
Whilst trying out the IRC-user's problem on the darcs repository, I ran into this.
% darcs get --to-match='exact "More fixes to unrecord."' ~/darcsImpl/stable
Copying patch 3831 of 3831... done!
darcs: failed to read patch in get_extra:
Sun May 22 13:40:59 GMT 2005 David Roundy
* extend revert_interactive test to do a trickier revert.
Perhaps this is a 'partial' repository?
Ouch! I should check the bug tracker and see if we have anything like it.
plink, Windows, ssh stuff
We had decided to reject my new ControlMaster patch because it was fragile, and then we weren't sure about the other patches which were essentially supposed to make Windows shut up when doing ssh stuff (so we don't get spurious plink -O error messages). I finally had a chance to sit down and review these patches. It seems that it is ok for us to apply them independently of the control master stuff. I have resubmitted the patches and made a request for review. Well, all patch submissions are implicitly a request for review, but I wanted to particularly call attention to this bit.
dates and the mystery bug
I had a filed a bug report for a
mysterious issue involving the darcs repository inventory. Juliusz looked into it and found that it was due to a cleanDate patch. The basic problem is that PatchInfo.make_filename uses
cleanDate
to parse the date info in old-school patches. Old school patches write their dates in human-readable format, which is rather useless because it's just an internal representation and harder to parse.
Case in point, the mystery bug. I had submitted a patch to fix issue173 (timezones not recognised correctly)... what the patch does is to correct
cleanDate
so that dates are converted to UTC before being printed out. I think this is the correct behaviour, because otherwise, timezones are just truncated; which would be very bad because somebody reading the time string in would have little choice but to assume UTC.
So I think my patch is correct: but it breaks old patches, because the patch in question was dated
Sun Sep 21 11:57:26 EDT 2003
, which is intepreted
before: 2003-09-21 11:57 (wrong!)
after: 2003-09-21 15:57 (EDT is UTC-4, so add 4 back)
Unfortunately, the filename generated was using the incorrect time (2003-09-21 11:57). Ick. So what I proposed is that we continue supporting this broken behaviour (it doesn't really matter anyway, it's just what filename we give to it), by using
showIsoDateTime.readDate
and a small comment saying that this is wrong, but we don't really care.
Anyway, I hope I haven't simply confused myself. I think this analysis of things is correct, maybe the next action is wrong, but I'm pretty sure that's where things went wrong. Sometimes seems that it's impossible to program a computer without being super anal about everything.
patch bundles
I have one particularly bad habit when submitting patches, and that is grouping my patches into thematic bundles, rather than dependency bundles. Why is this bad? It causes confusion! It makes people think that certain patches go together, when in fact the only thing about them is that they are related to common themes, like "date parsing". I need to get a better eye for grouping my patches so that things work out smoother for everybody.
Stayed home from work yesterday, giving myself the excuse that I was sick. Probably should have gone to work instead, because it turns out I was feeling ok enough to do a bit of darcs-hacking. Also played hooky a bit this morning (it's 11:06), but overall, I'm glad to have spent time getting this stuff out of the way... clears my head, let's me focus on other things.
windows and weird filenames
A user on IRC was trying to work around
issue53 where Windows enforces a bunch of naming conventions like "don't start your filenames with AUX". I don't think I personally am going to do something about this, but my proposal is that, when we are running on Windows, we should treat patches that affect such files as affecting some other file instead, one with a translated name. So if we have a patch that affects AUX.dtd, we should apply the changes to -AUX.dtd or something; and print out a warning saying what we have done and why. This solution at least allows the patches to apply, but it does have some problems of its own, like (1) what if a file with the translated name already exists? (2) what if the user tries to modify the translated file? Do we want to recognise that? [I'd say, we probably should, but rather than let the user record changes, we just say "Whoops! you don't want to touch this file. Please mv it to something safe instead."] Mmmhh. I dunno if this is going to work, or if it's just going to make things super-complicated and introduce a bunch of random bloat. Well, in any case, we should at least
detect the problem and tell the user about it. I suppose simply ignoring such files (and printing a warning) is just as good, and much simpler to boot. (If we were to ignore them, we should at least try to handle the mv, though, although that too is complicated)
get --to-match
Whilst trying out the IRC-user's problem on the darcs repository, I ran into this.
% darcs get --to-match='exact "More fixes to unrecord."' ~/darcsImpl/stable
Copying patch 3831 of 3831... done!
darcs: failed to read patch in get_extra:
Sun May 22 13:40:59 GMT 2005 David Roundy
* extend revert_interactive test to do a trickier revert.
Perhaps this is a 'partial' repository?
Ouch! I should check the bug tracker and see if we have anything like it.
plink, Windows, ssh stuff
We had decided to reject my new ControlMaster patch because it was fragile, and then we weren't sure about the other patches which were essentially supposed to make Windows shut up when doing ssh stuff (so we don't get spurious plink -O error messages). I finally had a chance to sit down and review these patches. It seems that it is ok for us to apply them independently of the control master stuff. I have resubmitted the patches and made a request for review. Well, all patch submissions are implicitly a request for review, but I wanted to particularly call attention to this bit.
dates and the mystery bug
I had a filed a bug report for a
mysterious issue involving the darcs repository inventory. Juliusz looked into it and found that it was due to a cleanDate patch. The basic problem is that PatchInfo.make_filename uses
cleanDate
to parse the date info in old-school patches. Old school patches write their dates in human-readable format, which is rather useless because it's just an internal representation and harder to parse.
Case in point, the mystery bug. I had submitted a patch to fix issue173 (timezones not recognised correctly)... what the patch does is to correct
cleanDate
so that dates are converted to UTC before being printed out. I think this is the correct behaviour, because otherwise, timezones are just truncated; which would be very bad because somebody reading the time string in would have little choice but to assume UTC.
So I think my patch is correct: but it breaks old patches, because the patch in question was dated
Sun Sep 21 11:57:26 EDT 2003
, which is intepreted
before: 2003-09-21 11:57 (wrong!)
after: 2003-09-21 15:57 (EDT is UTC-4, so add 4 back)
Unfortunately, the filename generated was using the incorrect time (2003-09-21 11:57). Ick. So what I proposed is that we continue supporting this broken behaviour (it doesn't really matter anyway, it's just what filename we give to it), by using
showIsoDateTime.readDate
and a small comment saying that this is wrong, but we don't really care.
Anyway, I hope I haven't simply confused myself. I think this analysis of things is correct, maybe the next action is wrong, but I'm pretty sure that's where things went wrong. Sometimes seems that it's impossible to program a computer without being super anal about everything.
patch bundles
I have one particularly bad habit when submitting patches, and that is grouping my patches into thematic bundles, rather than dependency bundles. Why is this bad? It causes confusion! It makes people think that certain patches go together, when in fact the only thing about them is that they are related to common themes, like "date parsing". I need to get a better eye for grouping my patches so that things work out smoother for everybody.
windows, ssh stuff, dates and the mystery bug
No comments:
Post a Comment