[Piglit] [PATCH] core: don't try to resolve the real path of the file

[Ken Phillis Jr]

I would suggest testing this a little bit more carefully. I know that
I specifically ran into problems with this because someone may be
running piglit in a situation where the user does not have write
permissions for the piglit install path. This is a common practice for
when users compile the program once and run the program across
multiple systems over a network.

On Mon, Feb 10, 2014 at 3:49 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Mon, Feb 10, 2014 at 2:41 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>> On Mon, Feb 10, 2014 at 2:30 AM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
>>> On Friday, February 07, 2014 10:22:48 PM Ilia Mirkin wrote:
>>>> On Fri, Feb 7, 2014 at 10:08 PM, Dylan Baker <baker.dylan.c at gmail.com>
>>> wrote:
>>>> > On Friday, February 07, 2014 09:42:05 PM Ilia Mirkin wrote:
>>>> >> This makes it possible to run the summary on e.g. compressed files or
>>>> >> otherwise piped in with the <( ... ) shell construct.
>>>> >>
>>>> >> There should be no difference between open() on a path before and after
>>>> >> the realpath call.
>>>> >>
>>>> >> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>>> >> ---
>>>> >>
>>>> >>  framework/core.py | 2 --
>>>> >>  1 file changed, 2 deletions(-)
>>>> >>
>>>> >> diff --git a/framework/core.py b/framework/core.py
>>>> >> index 45eea12..6a122f5 100644
>>>> >> --- a/framework/core.py
>>>> >> +++ b/framework/core.py
>>>> >>
>>>> >> @@ -647,8 +647,6 @@ def load_results(filename):
>>>> >>      "main"
>>>> >>
>>>> >>      """
>>>> >>
>>>> >> -    filename = os.path.realpath(filename)
>>>> >> -
>>>> >>
>>>> >>      try:
>>>> >>          with open(filename, 'r') as resultsfile:
>>>> >>              testrun = TestrunResult(resultsfile)
>>>> >
>>>> > I know that some people install piglit and add it's programs to their
>>>> > $PATH, is this going to break any of those use cases? It didn't seem to
>>>> > when I tested it, but some of those people might want to weigh in.
>>>>
>>>> I can't see how it would matter one way or another. Stuff like
>>>> realpath is to deal with people feeding symlinks/etc that end up
>>>> pointing outside of a tree (so you want to deny that for security
>>>> reasons). But perhaps there's something I'm missing...
>>>>
>>>>   -ilia
>>>
>>> That was the reason this was originally added IIRC. However, no one seems to
>>
>> The reason was to try to deny people the ability to use symlinks that
>> pointed outside of a fixed tree? Seem unlikely...
>
> The only two real reasons I can think of are (a) someone wasn't sure
> and just stuck it in for good measure, and (b) something really weird
> happens on e.g. windows. I think being able to run it with pipes
> overrides some really odd potential use-case we don't know about and
> can't think of. Besides if we check this in and someone complains,
> we'll find out the specifics and can work out something that works for
> everyone.
>
>>
>> The problem here though is that <(xzcat file.xz) gives it like
>> /dev/fd/63 which is a "fake" symlink, i.e. it reads as a symlink but
>> it points to a non-sensical location (a "pipe" object in /proc). And
>> open() doesn't work on that.
>>
>>> be complaining, your code seems fine, it doesn't break anything that I can
>>> see.
>>>
>>> Reviewed-by: Dylan Baker <baker.dylan.c at gmail.com>
>>
>> Thanks! I still don't have piglit commit access (perhaps it's time I
>> asked for it), could you check this in?
>
> Actually looks like I _am_ in the piglit group. Ha! So I'm going to
> push this and the other commit shortly.
>
>   -ilia
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit