On Fri, Aug 27, 2010 at 3:25 PM, Eric Kow <[email protected]> wrote: > > > According to the comments in URL.hs, path like "C:blah-blah-blah" is > > considered absolute under Windows with rationale that mentioning drive > name > > in path makes it absolute. In fact, it is not the case, since "C:blah" is > > relative to current working dir on drive C: and only stuff like "C:\blah" > > would be absolute. > > > > So I treated that clause as a bug lying in waiting and removed it. > > It may even be worth filing a ticket for, not sure. > At the very least mentioning in the patch comments >
I have a strong suspicion that issue1240 is already a proper ticket for this. Unfortunately, right now I dont have access to Windows box capable of building bleeding-edge darcs in order to test it. My chain of reasoning: * I assume that isRelative has a latent bug that will fire on Windows in certain cases by treating relative paths as absolute * There is a single call site for isRelative - it's the simpleSubPath * simpleSubPath, in turn, is used in two places: - In Add.lhs to fix/check names of the files added to the repo - In Arguments.lhs, in function fixSubPaths, for processing command line arguments and detecting (im)proper directory names. * Now, this latent bug could not fire during "darcs add" since all filepaths processed there are generated by traversing directory tree, and they, by definition, are relative to the repo root and would not contain drive letters. * We are left with the case when user-supplied filepath is used as the repo name (for darcs get or push, or similar). Voila, that is what the issue1240 is about! So maybe reporter of the issue1240 could be prodded to produce the shell test harness and test whether this patch fixes that bug as well. Or maybe someone could build a win32 darcs for me :) > > One possibility actually is if isRelative is only used by RepoPath.hs to > just > nuke it altogether and have RepoPath do the reasoning via FilePath. There > we'll have to ask ourselves if at RepoPath time, we need to care about > remote > paths or not, which we probably don't > But we probably do! Since simpleSubPath (which calls isRelative) is used to process command line arguments as well (see above). Maybe the proper course of action would be to move simpleSubPath out of RepoPath into Arguments.lhs, and replace call to simpleSubPath in Add.lhs with something simplier. > > Summarizing: I still insist that my patch fixes the issue at hand, and > has > > no ill side-effects (pun intended). > > OK! So I'm a lot less nervous about this having slept on it (I realised > that > this is actually still quite surface level stuff that shouldn't affect any > of > the inner patch-level stuff). Let me see if I can find you a patch > reviewer. > > Meanwhile, could I abuse you to somehow make our path testing logic much > more > aggressive and evil? > Maybe :) What do you mean by that? -- Dmitry Astapov
_______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
