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 <imir...@alum.mit.edu> wrote: > On Mon, Feb 10, 2014 at 2:41 AM, Ilia Mirkin <imir...@alum.mit.edu> wrote: >> On Mon, Feb 10, 2014 at 2:30 AM, Dylan Baker <baker.dyla...@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.dyla...@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 <imir...@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.dyla...@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/piglit _______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
