distcheck fails: Permission denied: '../man/systemd.directives.xml

[martinpitt]

Since commit 747ca69 from PR #152 distcheck fails:

  GEN      ../man/systemd.directives.xml
Traceback (most recent call last):
  File "../tools/make-directive-index.py", line 320, in <module>
    with open(sys.argv[1], 'wb') as f:
PermissionError: [Errno 13] Permission denied: '../man/systemd.directives.xml'
Makefile:21408: recipe for target '../man/systemd.directives.xml' failed
make[5]: *** [../man/systemd.directives.xml] Error 1
Makefile:18568: recipe for target 'check-recursive' failed
make[4]: *** [check-recursive] Error 1
Makefile:19791: recipe for target 'check' failed
make[3]: *** [check] Error 2
Makefile:19706: recipe for target 'distcheck' failed
make[2]: *** [distcheck] Error 1

Reverting this fixes distcheck again. This is happening in our daily distcheck builds (e. g. https://launchpadlibrarian.net/209133087/buildlog_ubuntu-wily-amd64.systemd-upstream-qa_0.v220-20150614-63432f5-0~ubuntu15.10.1_BUILDING.txt.gz) but I also get it locally. This doesn't happen with normal make/make check, as I usually don't build in a separate build dir with read-only parent (which is what distcheck does). Missing $(build_dir) somewhere? @zonque, any idea?

[filbranden]

This one is tricky because I think we still want man/systemd.directives.xml to be included in the dist archive created by make dist, so the vpath mechanism will pick up that one and try to update it instead of the one in the build directory...

Not really sure what to suggest here...

[filbranden]

Considering man/systemd.directives.xml is already listed in CLEANFILES, maybe the sanest solution is to just remove that explicit dependency and rely on make clean to regenerate both files...

The dependency of man/custom-entities.ent on configure.ac is bogus anyways, because if I re-run ./configure with different prefix and rootprefix etc. paths then custom-entities.ent would have to be regenerated but it won't be since none of its dependencies (i.e. configure.ac) actually changed... We're relying on make clean for that case, so we should probably be relying on it for this case as well...

So I'd say just revert 747ca69 here.

Opinions?

[zonque]

Hmm, how does your setup look like, exactly? I don't see how make distcheck operates on a read-only source dir, and in a setup that does that (configure --enable-compat-libs --srcdir=../, with .. being owned by root:root), I get all sorts of issues because for instance, make update-man-list wants to generate ../Makefile-man.tmp in the read-only source dir.

How's that read-only srcdir set up for you, exactly?

@filbranden I'm not convinced generating files on make clean is a good idea. I'd like to fully understand what's going on here.

[martinpitt]

How's that read-only srcdir set up for you, exactly?

There's nothing magic to that -- "make distcheck" from current automake (1.14 here) does that. It unpacks the tarball into systemd-220/, makes the entire dir readonly (i. e. -R a-w), and configures/builds in systemd-220/build/.

I didn't try --srcdir=../ -- I thought the standard way of building out of tree was something like

mkdir build
cd build
../configure [...]

Anyway, I'm quite surprised that this doesn't affect semaphore. I even get a build failure with the above, i. e. when the source tree is not read-only (the error just looks differently).

[filbranden]

@zonque Sorry if my suggestion wasn't clear, what I meant was that on make clean both files will be removed so both of them will be regenerated on the next make.

But now I realize that is fragile since there is no dependency to enforce ordering, maybe that could be solved by using a | style dependency to say that if both are built then the XML must be built after the .ent?

man/systemd.directives.xml: $(top_srcdir)/tools/make-directive-index.py $(SOURCE_XML_FILES) | man/custom-entities.ent

I haven't really tested that myself (kind of swamped right now).

[zonque]

I'm still trying to get my head around what the actual problem is. On my F22 machine (automake 1.15), I simply cannot reproduce the issue you're seeing, @martinpitt. I do see a different one though, which is related to intltool-update:

The following files contain translations and are currently not in use. Please
consider adding these to the POTFILES.in file, located in the po/ directory.

    sub/src/core/org.freedesktop.systemd1.policy.in

If some of these files are left out on purpose then please add them to
POTFILES.skip instead of POTFILES.in. A file 'missing' containing this list
of left out files has been written in the current directory.
Please report to http://github.com/systemd/systemd/issues

Also, I wonder how a build would possibly succeed in a read-only folder, where binaries cannot be created. @martinpitt, could you paste-dump your build error from these command sequence?

mkdir build
cd build
../configure [...]

I'll keep digging.

[martinpitt]

@zonque: I get that error about org.freedesktop.systemd1.policy.in as well, I haven't investigated that yet. This is a separate issue, though.

Also, I wonder how a build would possibly succeed in a read-only folder,

It doesn't. systemd-220/ is all read-only except the systemd-220/build subdirectory, of course. There's no Debian package for automake 1.15 yet, perhaps the behaviour changed there.

could you paste-dump your build error from these command sequence?

I configured with

$ ../configure CFLAGS="-g -O0" --prefix=/usr --includedir=\${prefix}/include -sysconfdir=/etc --localstatedir=/var --libdir=\${prefix}/lib/x86_64-linux-gnu --libexecdir=\${prefix}/lib/x86_64-linux-gnu --with-rootprefix= --with-rootlibdir=/lib/x86_64-linux-gnu --with-sysvinit-path=/etc/init.d --with-sysvrcnd-path=/etc --with-debug-shell=/bin/bash --with-telinit=/lib/sysvinit/telinit --enable-split-usr --disable-qrencode --disable-microhttpd --disable-chkconfig --disable-gnutls --disable-libcurl --disable-libidn --disable-elfutils --disable-vconsole --disable-sysusers --disable-firstboot --disable-coredump --disable-xkbcommon --disable-myhostname --disable-importd --disable-libiptc "--with-ntp-servers=0.debian.pool.ntp.org 1.debian.pool.ntp.org 2.debian.pool.ntp.org 3.debian.pool.ntp.org" --with-system-uid-max=999 --with-system-gid-max=999 --enable-libcryptsetup --enable-selinux --enable-apparmor --enable-introspection --enable-gtk-doc --enable-compat-libs --enable-hwdb PYTHON=python3 >> /tmp/log 2>&1 && make -j4 >> /tmp/log 2>&1

which fails with

 GEN      man/systemd.directives.xml
Traceback (most recent call last):
  File "../tools/make-directive-index.py", line 313, in make_page
    _extract_directives(directive_groups, formatting, page)
  File "../tools/make-directive-index.py", line 191, in _extract_directives
    t = xml_parse(page)
  File "/home/martin/upstream/systemd/tools/xml_helper.py", line 30, in xml_parse
    doc = tree.parse(page, _parser)
  File "lxml.etree.pyx", line 3301, in lxml.etree.parse (src/lxml/lxml.etree.c:72453)
  File "parser.pxi", line 1791, in lxml.etree._parseDocument (src/lxml/lxml.etree.c:105915)
  File "parser.pxi", line 1817, in lxml.etree._parseDocumentFromURL (src/lxml/lxml.etree.c:106214)
  File "parser.pxi", line 1721, in lxml.etree._parseDocFromFile (src/lxml/lxml.etree.c:105213)
  File "parser.pxi", line 1122, in lxml.etree._BaseParser._parseDocFromFile (src/lxml/lxml.etree.c:100163)
  File "parser.pxi", line 580, in lxml.etree._ParserContext._handleParseResultDoc (src/lxml/lxml.etree.c:94286)
  File "parser.pxi", line 690, in lxml.etree._handleParseResult (src/lxml/lxml.etree.c:95722)
  File "parser.pxi", line 618, in lxml.etree._raiseParseError (src/lxml/lxml.etree.c:94754)
OSError: Error reading file 'man/bootup.xml': failed to load external entity "man/bootup.xml"

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "../tools/make-directive-index.py", line 321, in <module>
    f.write(xml_print(make_page(*sys.argv[2:])))
  File "../tools/make-directive-index.py", line 315, in make_page
    raise ValueError("failed to process " + page)
ValueError: failed to process man/bootup.xml

Complete log at http://paste.ubuntu.com/11730456/ .

This could be the same root cause as distcheck failing, but it looks a little different and in this case the source tree is writable; so it likely gets a little further.

What's interesting is that I don't get an error with the configure invocation that autogen.sh recommends, i. e. this seem specific to a particular build option of the above. I'll track this down, that's more likely than a different automake version.

[martinpitt]

The only change compared to the autogen.sh recommended configure options that makes this happen is to configure for python3:

rm -rf build; mkdir build; cd build; ../configure CFLAGS='-g -O0 -ftrapv' --enable-compat-libs --enable-kdbus --sysconfdir=/etc --localstatedir=/var --libdir=/usr/lib  --with-rootprefix=/ --with-rootlibdir=/lib PYTHON=python3

I don't get python support in my build chroot with the default Python 2, as I don't have python-lxml installed (just python3-lxml):

checking for python lxml module... no
configure: WARNING: *** python support requires python-lxml module installed
configure: WARNING: *** python support not found, some documentation cannot be built

When I do install python-lxml, then the docs do get built, and I get the error from this report again. @zonque, can you please check if you get checking for python lxml module... yes, and if not, install lxml?

[zonque]

Ok, we're looking at too many unrelated issues here.

1. The org.freedesktop.systemd1.policy.in thing is a bug in intltool-update that stumbles over files generated during make distcheck. Not sure what to do about that, other than disabling this check for now.
2. The error quoted above (OSError: Error reading file 'man/bootup.xml': failed to load external entity "man/bootup.xml") is caused by a missing substitution of SOURCE_XML_FILES to make them relative to $(top_srcdir).
3. The PermissionError: [Errno 13] Permission denied: '../man/systemd.directives.xml' error is caused by a wrong Makefile dependency. man/systemd.directives.xml is actually part of EXTRA_DIST, so it is pre-generated and part of the tarball. Hence there's no need to generate it again during make distcheck.

Could you try the two upper-most patches from this tree and tell me if they solve your issues?

https://github.com/zonque/systemd/commits/dist

[martinpitt]

The first patch for 2. ("build-sys: make SOURCE_XML_FILES array relative to $(top_srcdir)") looks good, and it indeed fixes the out-of-tree build.

The second ("prefix man/custom-entities.ent dependency with $(top_srcdir)") doesn't though it breaks a normal out-of-tree build as man/custom-entities.ent does not exist in the srcdir; it's a built file, and should never be (and currently isn't) added to a dist tarball as its contents by nature is specific to your ./configure options.

I think the problem here is that this rule makes a disted file man/systemd.directives.xml depend on a generated file man/custom-entities.ent, which can't work. Everything that depends on a built file transitively also needs to be built and can't be disted.

I committed a potential fix to my branch: martinpitt@b40d3a6 (this branch also contains your first patch). With that, out-of-tree builds and distcheck both work well. I also tested a build without lxml, i. e. with limited documentation build.

OOI, can you reproduce the failure now with having python-lxml available or building for python 3?

[zonque]

Hmm, so with these changes, we would require the build host to generate man/systemd.directives.xml itself and not ship it in the tarball. That means that generating man pages depends on python. Not sure if that's acceptable, but I fail to see any other option right now.

@kaysievers, @poettering - any opinion?
@martinpitt Could you open a PR with the two patches from your tree for easier review?

[martinpitt]

We still dist the generated html and .1 manpage, just not the built xml source. So it's rather the other way around -- without python/lxml you don't get correct manpages according to your ./configure, but you get the pre-built ones from the tarball. Which is a reasonable compromise IMHO.

Will open a PR.