| Message ID | 20200403165422.21714-1-philmd@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PATCH-for-5.0?] configure: Do not leave sphinx in-tree artifacts | expand |
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
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 >
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?
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.
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!
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.
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(-)