All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iotests: fix source directory location
@ 2022-05-26  0:25 John Snow
  2022-05-26  7:54 ` Daniel P. Berrangé
  0 siblings, 1 reply; 7+ messages in thread
From: John Snow @ 2022-05-26  0:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, Kevin Wolf, Hanna Reitz, John Snow, Paolo Bonzini

If you invoke the check script from outside of the tests/qemu-iotests
directory, the directories initialized as source_iotests and
build_iotests will be incorrect.

We can use the location of the source file itself to be more accurate.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qemu-iotests/testenv.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index a864c74b123..9b0f01e84db 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -217,10 +217,10 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
             self.build_iotests = os.path.dirname(os.path.abspath(sys.argv[0]))
         else:
             # called from the source tree
-            self.source_iotests = os.getcwd()
+            self.source_iotests = str(Path(__file__, '..').resolve())
             self.build_iotests = self.source_iotests
 
-        self.build_root = os.path.join(self.build_iotests, '..', '..')
+        self.build_root = str(Path(self.build_iotests, '../..').resolve())
 
         self.init_directories()
         self.init_binaries()
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] iotests: fix source directory location
  2022-05-26  0:25 [PATCH] iotests: fix source directory location John Snow
@ 2022-05-26  7:54 ` Daniel P. Berrangé
  2022-05-26 14:21   ` John Snow
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel P. Berrangé @ 2022-05-26  7:54 UTC (permalink / raw)
  To: John Snow; +Cc: qemu-devel, qemu-block, Kevin Wolf, Hanna Reitz, Paolo Bonzini

On Wed, May 25, 2022 at 08:25:12PM -0400, John Snow wrote:
> If you invoke the check script from outside of the tests/qemu-iotests
> directory, the directories initialized as source_iotests and
> build_iotests will be incorrect.
> 
> We can use the location of the source file itself to be more accurate.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/qemu-iotests/testenv.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
> index a864c74b123..9b0f01e84db 100644
> --- a/tests/qemu-iotests/testenv.py
> +++ b/tests/qemu-iotests/testenv.py
> @@ -217,10 +217,10 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
>              self.build_iotests = os.path.dirname(os.path.abspath(sys.argv[0]))
>          else:
>              # called from the source tree
> -            self.source_iotests = os.getcwd()
> +            self.source_iotests = str(Path(__file__, '..').resolve())

Path(__file__).parent

>              self.build_iotests = self.source_iotests
>  
> -        self.build_root = os.path.join(self.build_iotests, '..', '..')
> +        self.build_root = str(Path(self.build_iotests, '../..').resolve())

Path(self.build_iotests).parent.parent

to be portable

>  
>          self.init_directories()
>          self.init_binaries()
> -- 
> 2.34.1
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] iotests: fix source directory location
  2022-05-26  7:54 ` Daniel P. Berrangé
@ 2022-05-26 14:21   ` John Snow
  2022-05-26 15:20     ` John Snow
  2022-05-27 16:29     ` Kevin Wolf
  0 siblings, 2 replies; 7+ messages in thread
From: John Snow @ 2022-05-26 14:21 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: qemu-devel, Qemu-block, Kevin Wolf, Hanna Reitz, Paolo Bonzini

[-- Attachment #1: Type: text/plain, Size: 1750 bytes --]

On Thu, May 26, 2022, 3:54 AM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Wed, May 25, 2022 at 08:25:12PM -0400, John Snow wrote:
> > If you invoke the check script from outside of the tests/qemu-iotests
> > directory, the directories initialized as source_iotests and
> > build_iotests will be incorrect.
> >
> > We can use the location of the source file itself to be more accurate.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  tests/qemu-iotests/testenv.py | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/qemu-iotests/testenv.py
> b/tests/qemu-iotests/testenv.py
> > index a864c74b123..9b0f01e84db 100644
> > --- a/tests/qemu-iotests/testenv.py
> > +++ b/tests/qemu-iotests/testenv.py
> > @@ -217,10 +217,10 @@ def __init__(self, imgfmt: str, imgproto: str,
> aiomode: str,
> >              self.build_iotests =
> os.path.dirname(os.path.abspath(sys.argv[0]))
> >          else:
> >              # called from the source tree
> > -            self.source_iotests = os.getcwd()
> > +            self.source_iotests = str(Path(__file__, '..').resolve())
>
> Path(__file__).parent
>
> >              self.build_iotests = self.source_iotests
> >
> > -        self.build_root = os.path.join(self.build_iotests, '..', '..')
> > +        self.build_root = str(Path(self.build_iotests,
> '../..').resolve())
>
> Path(self.build_iotests).parent.parent
>
> to be portable
>

With windows? I think Path() is meant to be a fully portable class as-is,
but I'll double-check my assumption. I use ".." elsewhere in code already
checked in, so if it's a problem I ought to fix it everywhere.

[-- Attachment #2: Type: text/html, Size: 2461 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] iotests: fix source directory location
  2022-05-26 14:21   ` John Snow
@ 2022-05-26 15:20     ` John Snow
  2022-05-26 15:23       ` Daniel P. Berrangé
  2022-05-27 16:29     ` Kevin Wolf
  1 sibling, 1 reply; 7+ messages in thread
From: John Snow @ 2022-05-26 15:20 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: qemu-devel, Qemu-block, Kevin Wolf, Hanna Reitz, Paolo Bonzini

On Thu, May 26, 2022 at 10:21 AM John Snow <jsnow@redhat.com> wrote:
>
>
>
> On Thu, May 26, 2022, 3:54 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>>
>> On Wed, May 25, 2022 at 08:25:12PM -0400, John Snow wrote:
>> > If you invoke the check script from outside of the tests/qemu-iotests
>> > directory, the directories initialized as source_iotests and
>> > build_iotests will be incorrect.
>> >
>> > We can use the location of the source file itself to be more accurate.
>> >
>> > Signed-off-by: John Snow <jsnow@redhat.com>
>> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>> > ---
>> >  tests/qemu-iotests/testenv.py | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
>> > index a864c74b123..9b0f01e84db 100644
>> > --- a/tests/qemu-iotests/testenv.py
>> > +++ b/tests/qemu-iotests/testenv.py
>> > @@ -217,10 +217,10 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
>> >              self.build_iotests = os.path.dirname(os.path.abspath(sys.argv[0]))
>> >          else:
>> >              # called from the source tree
>> > -            self.source_iotests = os.getcwd()
>> > +            self.source_iotests = str(Path(__file__, '..').resolve())
>>
>> Path(__file__).parent
>>
>> >              self.build_iotests = self.source_iotests
>> >
>> > -        self.build_root = os.path.join(self.build_iotests, '..', '..')
>> > +        self.build_root = str(Path(self.build_iotests, '../..').resolve())
>>
>> Path(self.build_iotests).parent.parent
>>
>> to be portable
>
>
> With windows? I think Path() is meant to be a fully portable class as-is, but I'll double-check my assumption. I use ".." elsewhere in code already checked in, so if it's a problem I ought to fix it everywhere.

Found a Windows box, it works there too. Good enough?

--js



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] iotests: fix source directory location
  2022-05-26 15:20     ` John Snow
@ 2022-05-26 15:23       ` Daniel P. Berrangé
  0 siblings, 0 replies; 7+ messages in thread
From: Daniel P. Berrangé @ 2022-05-26 15:23 UTC (permalink / raw)
  To: John Snow; +Cc: qemu-devel, Qemu-block, Kevin Wolf, Hanna Reitz, Paolo Bonzini

On Thu, May 26, 2022 at 11:20:18AM -0400, John Snow wrote:
> On Thu, May 26, 2022 at 10:21 AM John Snow <jsnow@redhat.com> wrote:
> >
> >
> >
> > On Thu, May 26, 2022, 3:54 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >>
> >> On Wed, May 25, 2022 at 08:25:12PM -0400, John Snow wrote:
> >> > If you invoke the check script from outside of the tests/qemu-iotests
> >> > directory, the directories initialized as source_iotests and
> >> > build_iotests will be incorrect.
> >> >
> >> > We can use the location of the source file itself to be more accurate.
> >> >
> >> > Signed-off-by: John Snow <jsnow@redhat.com>
> >> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> >> > ---
> >> >  tests/qemu-iotests/testenv.py | 4 ++--
> >> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
> >> > index a864c74b123..9b0f01e84db 100644
> >> > --- a/tests/qemu-iotests/testenv.py
> >> > +++ b/tests/qemu-iotests/testenv.py
> >> > @@ -217,10 +217,10 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
> >> >              self.build_iotests = os.path.dirname(os.path.abspath(sys.argv[0]))
> >> >          else:
> >> >              # called from the source tree
> >> > -            self.source_iotests = os.getcwd()
> >> > +            self.source_iotests = str(Path(__file__, '..').resolve())
> >>
> >> Path(__file__).parent
> >>
> >> >              self.build_iotests = self.source_iotests
> >> >
> >> > -        self.build_root = os.path.join(self.build_iotests, '..', '..')
> >> > +        self.build_root = str(Path(self.build_iotests, '../..').resolve())
> >>
> >> Path(self.build_iotests).parent.parent
> >>
> >> to be portable
> >
> >
> > With windows? I think Path() is meant to be a fully portable class as-is, but I'll double-check my assumption. I use ".." elsewhere in code already checked in, so if it's a problem I ought to fix it everywhere.
> 
> Found a Windows box, it works there too. Good enough?

I don't mind

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] iotests: fix source directory location
  2022-05-26 14:21   ` John Snow
  2022-05-26 15:20     ` John Snow
@ 2022-05-27 16:29     ` Kevin Wolf
  2022-07-11 21:30       ` John Snow
  1 sibling, 1 reply; 7+ messages in thread
From: Kevin Wolf @ 2022-05-27 16:29 UTC (permalink / raw)
  To: John Snow
  Cc: Daniel P. Berrangé,
	qemu-devel, Qemu-block, Hanna Reitz, Paolo Bonzini

Am 26.05.2022 um 16:21 hat John Snow geschrieben:
> On Thu, May 26, 2022, 3:54 AM Daniel P. Berrangé <berrange@redhat.com>
> wrote:
> 
> > On Wed, May 25, 2022 at 08:25:12PM -0400, John Snow wrote:
> > > If you invoke the check script from outside of the tests/qemu-iotests
> > > directory, the directories initialized as source_iotests and
> > > build_iotests will be incorrect.
> > >
> > > We can use the location of the source file itself to be more accurate.
> > >
> > > Signed-off-by: John Snow <jsnow@redhat.com>
> > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> > > ---
> > >  tests/qemu-iotests/testenv.py | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tests/qemu-iotests/testenv.py
> > b/tests/qemu-iotests/testenv.py
> > > index a864c74b123..9b0f01e84db 100644
> > > --- a/tests/qemu-iotests/testenv.py
> > > +++ b/tests/qemu-iotests/testenv.py
> > > @@ -217,10 +217,10 @@ def __init__(self, imgfmt: str, imgproto: str,
> > aiomode: str,
> > >              self.build_iotests =
> > os.path.dirname(os.path.abspath(sys.argv[0]))
> > >          else:
> > >              # called from the source tree
> > > -            self.source_iotests = os.getcwd()
> > > +            self.source_iotests = str(Path(__file__, '..').resolve())
> >
> > Path(__file__).parent
> >
> > >              self.build_iotests = self.source_iotests
> > >
> > > -        self.build_root = os.path.join(self.build_iotests, '..', '..')
> > > +        self.build_root = str(Path(self.build_iotests,
> > '../..').resolve())
> >
> > Path(self.build_iotests).parent.parent
> >
> > to be portable
> >
> 
> With windows? I think Path() is meant to be a fully portable class as-is,
> but I'll double-check my assumption. I use ".." elsewhere in code already
> checked in, so if it's a problem I ought to fix it everywhere.

I don't see any potential problem with the second hunk because we're
dealing with the path of a directory there, but "regular_file.py/.."
looks a bit fishy to me and doesn't work if you ask the kernel. Is this
guaranteed to work in Python or is it an implementation detail of Path
that may change?

Kevin



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] iotests: fix source directory location
  2022-05-27 16:29     ` Kevin Wolf
@ 2022-07-11 21:30       ` John Snow
  0 siblings, 0 replies; 7+ messages in thread
From: John Snow @ 2022-07-11 21:30 UTC (permalink / raw)
  To: Kevin Wolf
  Cc: Daniel P. Berrangé,
	qemu-devel, Qemu-block, Hanna Reitz, Paolo Bonzini

[-- Attachment #1: Type: text/plain, Size: 2773 bytes --]

On Fri, May 27, 2022, 12:29 PM Kevin Wolf <kwolf@redhat.com> wrote:

> Am 26.05.2022 um 16:21 hat John Snow geschrieben:
> > On Thu, May 26, 2022, 3:54 AM Daniel P. Berrangé <berrange@redhat.com>
> > wrote:
> >
> > > On Wed, May 25, 2022 at 08:25:12PM -0400, John Snow wrote:
> > > > If you invoke the check script from outside of the tests/qemu-iotests
> > > > directory, the directories initialized as source_iotests and
> > > > build_iotests will be incorrect.
> > > >
> > > > We can use the location of the source file itself to be more
> accurate.
> > > >
> > > > Signed-off-by: John Snow <jsnow@redhat.com>
> > > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> > > > ---
> > > >  tests/qemu-iotests/testenv.py | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/tests/qemu-iotests/testenv.py
> > > b/tests/qemu-iotests/testenv.py
> > > > index a864c74b123..9b0f01e84db 100644
> > > > --- a/tests/qemu-iotests/testenv.py
> > > > +++ b/tests/qemu-iotests/testenv.py
> > > > @@ -217,10 +217,10 @@ def __init__(self, imgfmt: str, imgproto: str,
> > > aiomode: str,
> > > >              self.build_iotests =
> > > os.path.dirname(os.path.abspath(sys.argv[0]))
> > > >          else:
> > > >              # called from the source tree
> > > > -            self.source_iotests = os.getcwd()
> > > > +            self.source_iotests = str(Path(__file__,
> '..').resolve())
> > >
> > > Path(__file__).parent
> > >
> > > >              self.build_iotests = self.source_iotests
> > > >
> > > > -        self.build_root = os.path.join(self.build_iotests, '..',
> '..')
> > > > +        self.build_root = str(Path(self.build_iotests,
> > > '../..').resolve())
> > >
> > > Path(self.build_iotests).parent.parent
> > >
> > > to be portable
> > >
> >
> > With windows? I think Path() is meant to be a fully portable class as-is,
> > but I'll double-check my assumption. I use ".." elsewhere in code already
> > checked in, so if it's a problem I ought to fix it everywhere.
>
> I don't see any potential problem with the second hunk because we're
> dealing with the path of a directory there, but "regular_file.py/.."
> looks a bit fishy to me and doesn't work if you ask the kernel. Is this
> guaranteed to work in Python or is it an implementation detail of Path
> that may change?
>

... I apparently never hit send on this draft reply:

Good question, I don't know. I just know that when starting from __file__,
it seems to work in that manner. I used that trick when I added the
PYTHONPATH stuff directly into testenv a while back and never thought more
of it.

If it makes people uneasy to look at, I can just use .parent for the
Principle of Least Surprise.

[-- Attachment #2: Type: text/html, Size: 4263 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-07-11 21:32 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-26  0:25 [PATCH] iotests: fix source directory location John Snow
2022-05-26  7:54 ` Daniel P. Berrangé
2022-05-26 14:21   ` John Snow
2022-05-26 15:20     ` John Snow
2022-05-26 15:23       ` Daniel P. Berrangé
2022-05-27 16:29     ` Kevin Wolf
2022-07-11 21:30       ` John Snow

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.