[PATCH-for-5.0?] configure: Do not leave sphinx in-tree artifacts

[PATCH-for-5.0?] configure: Do not leave sphinx in-tree artifacts

[Philippe Mathieu-Daudé]

When ./configure checks the sphinx version is new enough, it leaves
the docs/sphinx/__pycache__/ directory. Avoid this by using the '-B'
option (don't write .py[co] files on import) via the
PYTHONDONTWRITEBYTECODE environment variable.

Reported-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 22870f3867..ed524399c7 100755
--- a/configure
+++ b/configure
@@ -4936,7 +4936,7 @@ has_sphinx_build() {
     # sphinx-build doesn't exist at all or if it is too old.
     mkdir -p "$TMPDIR1/sphinx"
     touch "$TMPDIR1/sphinx/index.rst"
-    "$sphinx_build" -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1
+    PYTHONDONTWRITEBYTECODE=yes "$sphinx_build" -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1
 }
 
 # Check if tools are available to build documentation.
-- 
2.21.1

Re: [PATCH-for-5.0?] configure: Do not leave sphinx in-tree artifacts

[Peter Maydell]

On Fri, 3 Apr 2020 at 17:54, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> When ./configure checks the sphinx version is new enough, it leaves
> the docs/sphinx/__pycache__/ directory. Avoid this by using the '-B'
> option (don't write .py[co] files on import) via the
> PYTHONDONTWRITEBYTECODE environment variable.
>
> Reported-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

This only happens for an in-tree build, right? I think in
that case you're kind of OK with having random stuff
left in the source tree... It seems easy enough to suppress
them though, so I guess we might as well.

thanks
-- PMM

Re: [PATCH-for-5.0?] configure: Do not leave sphinx in-tree artifacts

[Philippe Mathieu-Daudé]

On 4/3/20 6:57 PM, Peter Maydell wrote:
> On Fri, 3 Apr 2020 at 17:54, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> When ./configure checks the sphinx version is new enough, it leaves
>> the docs/sphinx/__pycache__/ directory. Avoid this by using the '-B'
>> option (don't write .py[co] files on import) via the
>> PYTHONDONTWRITEBYTECODE environment variable.
>>
>> Reported-by: Eric Blake <eblake@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> This only happens for an in-tree build, right?

Correct.

> I think in
> that case you're kind of OK with having random stuff
> left in the source tree... It seems easy enough to suppress
> them though, so I guess we might as well.

Here is the post where Eric commented it:
https://lists.gnu.org/archive/html/qemu-devel/2020-04/msg00631.html

> 
> thanks
> -- PMM
> 

Re: [PATCH-for-5.0?] configure: Do not leave sphinx in-tree artifacts

[Eric Blake]

On 4/3/20 11:57 AM, Peter Maydell wrote:
> On Fri, 3 Apr 2020 at 17:54, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> When ./configure checks the sphinx version is new enough, it leaves
>> the docs/sphinx/__pycache__/ directory. Avoid this by using the '-B'
>> option (don't write .py[co] files on import) via the
>> PYTHONDONTWRITEBYTECODE environment variable.
>>
>> Reported-by: Eric Blake <eblake@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> This only happens for an in-tree build, right? I think in
> that case you're kind of OK with having random stuff
> left in the source tree... It seems easy enough to suppress
> them though, so I guess we might as well.

It happens in VPATH too - and what's more, in VPATH, it is still 
creating it under srcdir rather than builddir, which feels like 
unnecessary pollution.  I was trying to prove whether 'make distclean' 
got us back to a pristine state; this was one of the files that escaped 
'make distclean', so our choice is to either add it to the clean rules, 
or to avoid creating it in the first place.  I like the approach of not 
creating it in the first place :)

Reviewed-by: Eric Blake <eblake@redhat.com>

That said,

> +    PYTHONDONTWRITEBYTECODE=yes "$sphinx_build" -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1

is now even a longer line; is it worth adding \-newline to split it up?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH-for-5.0?] configure: Do not leave sphinx in-tree artifacts

[Eric Blake]

On 4/3/20 12:37 PM, Eric Blake wrote:
> On 4/3/20 11:57 AM, Peter Maydell wrote:
>> On Fri, 3 Apr 2020 at 17:54, Philippe Mathieu-Daudé 
>> <philmd@redhat.com> wrote:
>>>
>>> When ./configure checks the sphinx version is new enough, it leaves
>>> the docs/sphinx/__pycache__/ directory. Avoid this by using the '-B'
>>> option (don't write .py[co] files on import) via the
>>> PYTHONDONTWRITEBYTECODE environment variable.
>>>
>>> Reported-by: Eric Blake <eblake@redhat.com>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>
>> This only happens for an in-tree build, right? I think in
>> that case you're kind of OK with having random stuff
>> left in the source tree... It seems easy enough to suppress
>> them though, so I guess we might as well.
> 
> It happens in VPATH too - and what's more, in VPATH, it is still 
> creating it under srcdir rather than builddir, which feels like 
> unnecessary pollution.  I was trying to prove whether 'make distclean' 
> got us back to a pristine state; this was one of the files that escaped 
> 'make distclean', so our choice is to either add it to the clean rules, 
> or to avoid creating it in the first place.  I like the approach of not 
> creating it in the first place :)
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>

Hmm, I spoke early.  Your patch only addresses the pollution during 
'./configure'.  But running 'make' (even in a VPATH build) equally 
creates the same pollution.  Which means we really ought to be cleaning 
it up during 'make distclean' rather than just trying to make 
'./configure' clever.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH-for-5.0?] configure: Do not leave sphinx in-tree artifacts

[Philippe Mathieu-Daudé]

On 4/3/20 7:49 PM, Eric Blake wrote:
> On 4/3/20 12:37 PM, Eric Blake wrote:
>> On 4/3/20 11:57 AM, Peter Maydell wrote:
>>> On Fri, 3 Apr 2020 at 17:54, Philippe Mathieu-Daudé 
>>> <philmd@redhat.com> wrote:
>>>>
>>>> When ./configure checks the sphinx version is new enough, it leaves
>>>> the docs/sphinx/__pycache__/ directory. Avoid this by using the '-B'
>>>> option (don't write .py[co] files on import) via the
>>>> PYTHONDONTWRITEBYTECODE environment variable.
>>>>
>>>> Reported-by: Eric Blake <eblake@redhat.com>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>
>>> This only happens for an in-tree build, right? I think in
>>> that case you're kind of OK with having random stuff
>>> left in the source tree... It seems easy enough to suppress
>>> them though, so I guess we might as well.
>>
>> It happens in VPATH too - and what's more, in VPATH, it is still 
>> creating it under srcdir rather than builddir, which feels like 
>> unnecessary pollution.  I was trying to prove whether 'make distclean' 
>> got us back to a pristine state; this was one of the files that 
>> escaped 'make distclean', so our choice is to either add it to the 
>> clean rules, or to avoid creating it in the first place.  I like the 
>> approach of not creating it in the first place :)
>>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> Hmm, I spoke early.  Your patch only addresses the pollution during 
> './configure'.  But running 'make' (even in a VPATH build) equally 
> creates the same pollution.  Which means we really ought to be cleaning 
> it up during 'make distclean' rather than just trying to make 
> './configure' clever.

Oh I only checked ./configure indeed, sorry. Thanks for testing!