Geoffrey Martin-Noble via llvm-dev
2021-Dec-15 20:38 UTC
[llvm-dev] RFC: Support relative `--path` arguments to lit
Any objections? I've sent the patch for review and will land next week if it's accepted and there isn't any other discussion. On Thu, Dec 9, 2021 at 5:46 PM Mehdi AMINI <joker.eph at gmail.com> wrote:> > > On Thu, Dec 9, 2021 at 5:42 PM Geoffrey Martin-Noble via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> I noticed that the lit `--path` argument currently accepts relative >> paths, but the way it operates with them is pretty broken. It differs >> between external shell and internal shell because the path resolution is >> done with a different working directory. With the internal shell, it's >> resolved relative to the directory from which lit is executed, whereas with >> the external shell it's resolved relative to where the test case is >> executed. To make matters worse, using the internal shell the file path >> to binaries looked up with the `which` method is returned relative to the >> directory from which lit is executed, but then executed from the test >> execution directory. That means that relative --path arguments when using >> the internal shell result in a `[Errno 2] No such file or directory` error >> instead of the expected `command not found`. >> >> I propose that `--path` arguments be resolved as absolute paths as part >> of command line argument parsing. So relative paths passed to `--path` >> would be consistently interpreted as relative to the directory from which >> lit was invoked, which seems the least surprising behavior. To address >> other potential edge cases from the `which` method returning relative >> paths, I also propose modifying it to return absolute paths, which matches >> the behavior of the unix function from which it gets its name. >> > > > FWIW that makes sense to me! > > Thanks for the clear explanation :) > > > -- > Mehdi > > > >> >> https://reviews.llvm.org/D115486 is a patch making this change. >> >> Thanks, >> Geoffrey >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20211215/e6b1d55f/attachment.html>
Reid Kleckner via llvm-dev
2021-Dec-16 22:46 UTC
[llvm-dev] RFC: Support relative `--path` arguments to lit
Ship it! On Wed, Dec 15, 2021 at 12:39 PM Geoffrey Martin-Noble via llvm-dev < llvm-dev at lists.llvm.org> wrote:> Any objections? I've sent the patch for review and will land next week if > it's accepted and there isn't any other discussion. > > On Thu, Dec 9, 2021 at 5:46 PM Mehdi AMINI <joker.eph at gmail.com> wrote: > >> >> >> On Thu, Dec 9, 2021 at 5:42 PM Geoffrey Martin-Noble via llvm-dev < >> llvm-dev at lists.llvm.org> wrote: >> >>> I noticed that the lit `--path` argument currently accepts relative >>> paths, but the way it operates with them is pretty broken. It differs >>> between external shell and internal shell because the path resolution is >>> done with a different working directory. With the internal shell, it's >>> resolved relative to the directory from which lit is executed, whereas with >>> the external shell it's resolved relative to where the test case is >>> executed. To make matters worse, using the internal shell the file path >>> to binaries looked up with the `which` method is returned relative to >>> the directory from which lit is executed, but then executed from the test >>> execution directory. That means that relative --path arguments when using >>> the internal shell result in a `[Errno 2] No such file or directory` error >>> instead of the expected `command not found`. >>> >>> I propose that `--path` arguments be resolved as absolute paths as part >>> of command line argument parsing. So relative paths passed to `--path` >>> would be consistently interpreted as relative to the directory from which >>> lit was invoked, which seems the least surprising behavior. To address >>> other potential edge cases from the `which` method returning relative >>> paths, I also propose modifying it to return absolute paths, which matches >>> the behavior of the unix function from which it gets its name. >>> >> >> >> FWIW that makes sense to me! >> >> Thanks for the clear explanation :) >> >> >> -- >> Mehdi >> >> >> >>> >>> https://reviews.llvm.org/D115486 is a patch making this change. >>> >>> Thanks, >>> Geoffrey >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>> >> _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20211216/f2cb82d6/attachment.html>