[PATCH] PR21482: long paths on Windows

Tue Nov 11 21:14:09 PST 2014

On Tue, Nov 11, 2014 at 8:52 PM, Robinson, Paul <
Paul_Robinson at playstation.sony.com> wrote:

> New patch attached.  Threw together . and .. elimination for Michael.
> Extra test for Rafael, although I had to #include <windows.h> in the
> unittest to make the necessary Windows APIs available. (Testing a relative
> path requires setting the current directory to a known writeable place,
> which means calling Windows APIs because path/fs don't have them.)

There's a pair of raw chdir calls in libTooling, and also in
unittests/Support/LockFileManagerTest.cpp. Maybe it's time libSupport grows
a chdir?

+  ASSERT_NO_ERROR(fs::remove(Twine("b")));

Twine should have an implicit constructor for const char *. Same goes for
other places where you explicitly use foo(Twine(x)).

-- Sean Silva

>   If it's
> going overboard to do that, let me know and I can take the relative-path
> test back out; but actually I think it's useful to have it.
> --paulr
>
> > -----Original Message-----
> > From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> > bounces at cs.uiuc.edu] On Behalf Of Robinson, Paul
> > Sent: Tuesday, November 11, 2014 2:13 PM
> > To: Michael Spencer
> > Cc: llvm-commits at cs.uiuc.edu; Aaron Ballman
> > Subject: RE: [PATCH] PR21482: long paths on Windows
> >
> > > -----Original Message-----
> > > From: Michael Spencer [mailto:bigcheesegs at gmail.com]
> > > Sent: Tuesday, November 11, 2014 12:36 PM
> > > To: Robinson, Paul
> > > Cc: Aaron Ballman; Rafael Espíndola; llvm-commits at cs.uiuc.edu
> > > Subject: Re: [PATCH] PR21482: long paths on Windows
> > >
> > > On Tue, Nov 11, 2014 at 7:27 AM, Robinson, Paul
> > > <Paul_Robinson at playstation.sony.com> wrote:
> > > > New patch attached.
> > > >
> > > > I ended up not using make_absolute because it caused more string
> > > copying.
> > > > If there's some reason to consider it safer than current_path and
> > doing
> > > > my own pasting, I can do that instead, but only if y'all think it has
> > > > actual value.
> > > >
> > > > This changed the specific error returned by another fs test, which
> > > > seems reasonable.
> > > > --paulr
> > >
> > > This patch doesn't seem to be able to handle "..\foo". \\?\ disables
> > > expansion of . and ..
> >
> > Feh.
> >
> > I'm not able to find a pre-Windows8 API that does this canonicalization
> > on sufficiently long path names.  If anybody else has one I'm all ears.
> > Otherwise I guess I have to write my own.
> > --paulr
> >
> > >
> > > Other than that it looks fine.
> > >
> > > - Michael Spencer
> > >
> > > >
> > > >> -----Original Message-----
> > > >> From: Michael Spencer [mailto:bigcheesegs at gmail.com]
> > > >> Sent: Monday, November 10, 2014 4:04 PM
> > > >> To: Robinson, Paul
> > > >> Cc: Aaron Ballman; Rafael Espíndola; llvm-commits at cs.uiuc.edu
> > > >> Subject: Re: [PATCH] PR21482: long paths on Windows
> > > >>
> > > >> On Mon, Nov 10, 2014 at 3:05 PM, Robinson, Paul
> > > >> <Paul_Robinson at playstation.sony.com> wrote:
> > > >> >> -----Original Message-----
> > > >> >> From: Aaron Ballman [mailto:aaron.ballman at gmail.com]
> > > >> >> Sent: Monday, November 10, 2014 2:47 PM
> > > >> >> To: Rafael Espíndola
> > > >> >> Cc: Robinson, Paul; llvm-commits at cs.uiuc.edu
> > > >> >> Subject: Re: [PATCH] PR21482: long paths on Windows
> > > >> >>
> > > >> >> On Mon, Nov 10, 2014 at 5:43 PM, Rafael Espíndola
> > > >> >> <rafael.espindola at gmail.com> wrote:
> > > >> >> > static std::error_code widen_path(const Twine &Path8,
> > > >> >> >
> > > >> >> > widenPath
> > > >> >> >
> > > >> >> > Using  '\\?\'  also disables using / as a path separator, no?
> > > Don't
> > > >> we
> > > >> >> > have to check/assert that there is no / in Path8?
> > > >> >
> > > >> > So for paranoia I should translate all / to \ in the widened path?
> > I
> > > >> don't
> > > >> > see any existing utility to do that, but easy enough to invent
> one.
> > > >>
> > > >> sys::path::native(). in Path.h
> > > >>
> > > >> We also have make_absolute in FileSystem.h
> > > >>
> > > >> - Michael Spencer
> > > >>
> > > >> >
> > > >> >> >
> > > >> >> > +  size_t NLevels = ((248 - TmpLen) / 10) + 1;
> > > >> >> > +  const char *OneDir = "\\123456789";
> > > >> >> >
> > > >> >> > Use the size of OneDir instead of hard codding the 10.
> > > >> >> >
> > > >> >> > Aaron, this only uses '\\?\' to create long paths. They can
> > still
> > > be
> > > >> >> > accessed by regular tools by changing the current directory,
> no?
> > > >> >>
> > > >> >> Yes, relative paths aren't an issue. Just tools using absolute
> > paths
> > > >> >> that don't support the extended path namespace (which can happen
> > if
> > > >> >> the app assumes MAX_PATH is actually the max).
> > > >> >>
> > > >> >> ~Aaron
> > > >> >>
> > > >> >> >
> > > >> >> > On 10 November 2014 16:01, Robinson, Paul
> > > >> >> > <Paul_Robinson at playstation.sony.com> wrote:
> > > >> >> >> Support directory names longer than 248 characters on Windows.
> > > >> >> >>
> > > >> >> >> The normal Windows path limit is 260 characters; the limit for
> > > >> >> directory
> > > >> >> >> names is 248, to leave room for an 8.3 filename at the end.
> > > Adding
> > > >> the
> > > >> >> >> '\\?\' prefix greatly expands these limits.  Intercept path
> > names
> > > >> that
> > > >> >> >> we are about to pass to the Windows APIs and add the prefix if
> > > >> >> necessary.
> > > >> >> >>
> > > >> >> >> Also correct the comment about the state of the temporary
> > > directory
> > > >> >> used
> > > >> >> >> by the Support unittests.
> > > >> >> >> --paulr
> > > >> >> >>
> > > >> >> >>
> > > >> >> >> _______________________________________________
> > > >> >> >> llvm-commits mailing list
> > > >> >> >> llvm-commits at cs.uiuc.edu
> > > >> >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > > >> >> >>
> > > >> >
> > > >> > _______________________________________________
> > > >> > llvm-commits mailing list
> > > >> > llvm-commits at cs.uiuc.edu
> > > >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141111/6ebd49d3/attachment.html>