After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 775003 - Build failure in GNOME Continuous
Build failure in GNOME Continuous
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2016-11-24 13:49 UTC by Emmanuele Bassi (:ebassi)
Modified: 2017-02-13 12:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Do not require GTK-Doc to build nmcli (2.45 KB, patch)
2016-11-24 17:13 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
Do not require GTK-Doc to build nmcli (2.90 KB, patch)
2016-11-24 17:23 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
[patch] v4 (4.55 KB, patch)
2016-11-25 15:13 UTC, Thomas Haller
none Details | Review

Description Emmanuele Bassi (:ebassi) 2016-11-24 13:49:49 UTC
The following commit:

https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=104546cc211739dc2356d4a39d4ace2679812aff

broke the build in GNOME continuous because it assumes that NetworkManager will only be built in two scenarios:

 * from a tarball
 * from Git with `--enable-gtk-doc`

If the documentation is required, then `--enable-gtk-doc` should always be true; otherwise you just broke Continuous and any other build bots or development environments that use Git and don't want to spend precious time and resources installing a ton of dependencies in order to build the documentation.

Ideally, the commit should be reverted, or a better fix should be devised.
Comment 1 Emmanuele Bassi (:ebassi) 2016-11-24 13:51:37 UTC
I noticed that the commit in question is preceded by:

https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=f2afabe610d23fa5d5a38774dbd62c9112083622

which obviously makes it work if you let autogen.sh run the configure script, but environments such as Continuous and Jhbuild use NOCONFIGURE=1 to run the configure script in a separate build directory.
Comment 2 Emmanuele Bassi (:ebassi) 2016-11-24 14:01:19 UTC
Personally, I don't understand the rationale of forcing the build of the API reference documentation from Git, especially for the nm-cli utility, since all you really need is xsltproc.

An appropriate fix would be:

  - check for xsltproc in configure.ac
  - generate the setting-docs.c file from setting-docs.xml using xsltproc
  - decouple BUILD_SETTING_DOCS from the checks for the introspection Makefile and enable_gtk_doc
Comment 3 Thomas Haller 2016-11-24 14:57:40 UTC
Note that

1) building NM always wants to build nmcli. nmcli #includes clients/cli/settings-docs.c, which is either generated as part of BUILD_SETTING_DOCS or you get it in the dist-ed tarball.

2) in the past, trying to build nmcli without settings-docs.c would result in nmcli without help text. We don't think such a configuration is useful.


So, yes, the change is, if you build from git-tree you need the required developer tools to bootstrap the package (that is: to generate required docs).



you need xsltproc, but also the introspection stuff and pygobject.

Is there a problem with installing gtk-doc and configure with "--enable-gtk-doc --with-introspection"?
Comment 4 Emmanuele Bassi (:ebassi) 2016-11-24 16:36:51 UTC
(In reply to Thomas Haller from comment #3)
> Note that
> 
> 1) building NM always wants to build nmcli. nmcli #includes
> clients/cli/settings-docs.c, which is either generated as part of
> BUILD_SETTING_DOCS or you get it in the dist-ed tarball.
> 
> 2) in the past, trying to build nmcli without settings-docs.c would result
> in nmcli without help text. We don't think such a configuration is useful.

I agree, and that is why I think this coupling of nmcli settings documentation and the API reference is not necessary.
 
> you need xsltproc, but also the introspection stuff and pygobject.

But you don't need gtk-doc; you're just using gtk-doc and the introspection toggles as a shortcut to assume that you have Python, Perl, and xsltproc installed. This is unnecessary.

> Is there a problem with installing gtk-doc and configure with
> "--enable-gtk-doc --with-introspection"?

The issue is using `--enable-gtk-doc`; Continuous explicitly disables building with GTK-Doc enabled because:

 - the API references are not necessary in 99% of the cases
 - the dependency chain for GTK-Doc is long and hard to justify on a build server
 - xsltproc is single-threaded and blocking, which would make building Continuous slower and take longer, when we care about keeping the turnaround between change hitting Git and an image being built as short as possible

Right now, NM's configure.ac folds everything documentation-related to `--enable-gtk-doc`, including the dependency checks; this is unnecessary and just makes the build more complicated. I outlined a different approach in comment 2, which allows nm-cli to be documented appropriately while not requiring that everyone building from Git also spends time building the NM API reference.
Comment 5 Thomas Haller 2016-11-24 16:48:42 UTC
(In reply to Emmanuele Bassi (:ebassi) from comment #4)
> (In reply to Thomas Haller from comment #3)
> > 
> comment 2, which allows nm-cli to be documented appropriately while not
> requiring that everyone building from Git also spends time building the NM
> API reference.

indeed.

building settings-docs.c requires libnm/nm-property-docs.xml, which requires introspection and pygobject.
Comment 6 Emmanuele Bassi (:ebassi) 2016-11-24 17:13:07 UTC
Created attachment 340700 [details] [review]
Do not require GTK-Doc to build nmcli

We can simply check for xsltproc and depend on pygobject and
introspection.
Comment 7 Emmanuele Bassi (:ebassi) 2016-11-24 17:15:14 UTC
With the patch in attachment 340700 [details] [review] I was able to build NetworkManager with --disable-gtk-doc from a client Git checkout.
Comment 8 Emmanuele Bassi (:ebassi) 2016-11-24 17:23:53 UTC
Created attachment 340702 [details] [review]
Do not require GTK-Doc to build nmcli

We can simply check for xsltproc and depend on pygobject and
introspection.
Comment 9 Thomas Haller 2016-11-25 14:51:57 UTC
AC_MSG_ERROR(["--enable-introspection aims to build the

Spelling: the option is called "--with-introspection".



Regardless of that, the patch goes in the right direction, but it's not correct.
BUILD_SETTINGS_DOCS had the meaning of --enable-gtk-doc && --with-introspection. It needs more fixes to Makefile.am to make this happen.

  git clean -fdx &&
     NOCONFIGURE=x ./autogen.sh &&
     ./configure --disable-gtk-doc --with-introspection &&
     make dist
Comment 10 Thomas Haller 2016-11-25 15:13:05 UTC
Created attachment 340763 [details] [review]
[patch] v4

basically, with patch#3 (comment 8) most of the manuals are build as part of --with-introspection. And --with-introspection requires xsltproc.

If you follow that through, it makes no sense to support a --enable-gtk-doc without also --with-introspection.

So, I think v4 would be fine, except that we already build all the manuals as part of --with-introspection. Only the html doc is done by --enable-gtk-doc. It seems now the naming of the configure options no longer fits, but well.

What do you think?
Comment 11 Thomas Haller 2016-11-28 11:46:06 UTC
merged a modified version to master:

https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=d8dd4e8bc243aadf64afcfcd568b43deed8a9524

I think it works now well... closing as fixed.
Comment 12 Javier Jardón (IRC: jjardon) 2017-02-13 00:50:13 UTC
This seems to be still a problem: http://build.gnome.org/continuous/buildmaster/builds/2017/02/13/2/build/log-NetworkManager.txt
Comment 13 Thomas Haller 2017-02-13 09:11:46 UTC
(In reply to Javier Jardón (IRC: jjardon) from comment #12)
> This seems to be still a problem:
> http://build.gnome.org/continuous/buildmaster/builds/2017/02/13/2/build/log-
> NetworkManager.txt

Why do you think the new issue is the same as this bug? It's advisable not to resurrect closed bugs.


/usr/bin/xsltproc --output man/nmcli.1 --path man --xinclude --nonet --stringparam man.output.quietly 1 --stringparam funcsynopsis.style ansi --stringparam man.th.extra1.suppress 1 --stringparam man.authors.sec
I/O error : Attempt to load network entity http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl
warning: failed to load external entity "http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl"
cannot parse http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl
Makefile:17014: recipe for target 'man/nmcli.1' failed


seems you are lacking some docbook package that provides docbook.xsl.
Comment 14 Emmanuele Bassi (:ebassi) 2017-02-13 10:05:48 UTC
(In reply to Thomas Haller from comment #13)
> (In reply to Javier Jardón (IRC: jjardon) from comment #12)
> > This seems to be still a problem:
> > http://build.gnome.org/continuous/buildmaster/builds/2017/02/13/2/build/log-
> > NetworkManager.txt
> 
> Why do you think the new issue is the same as this bug? It's advisable not
> to resurrect closed bugs.
> 
> /usr/bin/xsltproc --output man/nmcli.1 --path man --xinclude --nonet
> --stringparam man.output.quietly 1 --stringparam funcsynopsis.style ansi
> --stringparam man.th.extra1.suppress 1 --stringparam man.authors.sec
> I/O error : Attempt to load network entity
> http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl
> warning: failed to load external entity
> "http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl"
> cannot parse
> http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl
> Makefile:17014: recipe for target 'man/nmcli.1' failed
> 
> 
> seems you are lacking some docbook package that provides docbook.xsl.

It should be possible to disable building the man pages altogether.

Anyway, I agree with Thomas: this is another bug entirely.
Comment 15 Javier Jardón (IRC: jjardon) 2017-02-13 12:37:24 UTC
Sorry, my fault

this is the new bug report opened by ebassi: https://bugzilla.gnome.org/show_bug.cgi?id=778551

I will update continuous to point to the correct bug report