Page MenuHomePhabricator

[llvm-ar] Accept file paths with windows format slashes
ClosedPublic

Authored by gbreynoo on Aug 5 2019, 6:17 AM.

Details

Summary

The internal representation of llvm-ar archives uses linux style slashes for paths, no matter the OS. In the case of windows this meant file paths input intending to match existing members would only match if linux style slashes where used. This change allows either slash direction to be input by the user.

This change includes removing an unnecessary call to normalisePath and moving the call of another.

Diff Detail

Event Timeline

gbreynoo created this revision.Aug 5 2019, 6:17 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 5 2019, 6:17 AM
jhenderson added inline comments.Aug 5 2019, 6:50 AM
llvm/test/tools/llvm-ar/windows-path.test
1

Nit: missing trailing full-stop.

MaskRay added inline comments.Aug 6 2019, 9:25 PM
llvm/test/tools/llvm-ar/windows-path.test
15

What does a\..\a\foo.txt do?

gbreynoo updated this revision to Diff 213884.Aug 7 2019, 8:01 AM
gbreynoo marked 2 inline comments as done.
gbreynoo added inline comments.
llvm/test/tools/llvm-ar/windows-path.test
15

I've added this to the test and behaviour is as expected.

rupprecht accepted this revision.Aug 7 2019, 11:45 AM

I think it'd be good to also write a test that verifies the internal representation is always forward slashes on windows by running FileCheck directly on the thin archive instead of using llvm-ar t to look at it (e.g. see thin-archive.test). Or do we already have a test for that?

This revision is now accepted and ready to land.Aug 7 2019, 11:45 AM
This revision was automatically updated to reflect the committed changes.

I think it'd be good to also write a test that verifies the internal representation is always forward slashes on windows by running FileCheck directly on the thin archive instead of using llvm-ar t to look at it (e.g. see thin-archive.test). Or do we already have a test for that?

mri-thin-archive.test does this but it may be best to have more cases in which the internal representation is checked.