modules: Add an 'hotdoc' module #4016

thiblahute · 2018-08-13T19:05:03Z

hotdoc: http://github.com/hotdoc/hotdoc/

ignatenkobrain · 2018-08-13T19:05:45Z

Flake8 detected 6 issues on b4597d0
Visit https://sider.review/gh/19784232/pull_requests/4016 to review the issues.

ignatenkobrain · 2018-08-13T22:19:08Z

mesonbuild/modules/hotdoc.py

+            from hotdoc.run_hotdoc import run
+        except Exception as e:
+            raise MesonException('hotdoc %s required but not found. (%s)' % (
+                MIN_HOTDOC_VERSION, d))


[Flake8]

[F821] undefined name 'd'

ignatenkobrain · 2018-08-13T22:19:09Z

mesonbuild/modules/hotdoc.py

+                return value
+
+            return os.path.relpath(os.path.join(state.environment.get_source_dir(), value),
+                state.environment.get_build_dir())


[Flake8]

[E128] continuation line under-indented for visual indent

ignatenkobrain · 2018-08-13T22:19:09Z

mesonbuild/modules/hotdoc.py

+            raise MesonException('hotdoc executable not found')
+
+        try:
+            from hotdoc.run_hotdoc import run


[Flake8]

[F401] 'hotdoc.run_hotdoc.run' imported but unused

ignatenkobrain · 2018-08-13T22:19:10Z

mesonbuild/modules/hotdoc.py

+                                 ' required for the project name.')
+
+        project_name = args[0]
+        def file_to_path(value):


[Flake8]

[E306] expected 1 blank line before a nested definition, found 0

ignatenkobrain · 2018-08-13T22:19:11Z

mesonbuild/modules/hotdoc.py

+
+
+def initialize(interpreter):
+    return HotDocModule(interpreter)


[Flake8]

[W292] no newline at end of file

thiblahute · 2018-08-14T13:25:04Z

@jpakkane Could you please rebuild your CI docker image with the changes from that PR ?

MathieuDuponchelle · 2018-08-14T13:40:26Z

test cases/frameworks/23 hotdoc/meson.build

@@ -0,0 +1,9 @@
+project('hotdoc', 'c')


Should probably specify a version here, then use meson.project_version() in generate_doc()

Would we want that to be automatic if project_version is not specified?

That sounds like a good idea, but it might lead to confusing bugs in practice, as most projects would want to use the major version here

jpakkane · 2018-08-14T17:24:31Z

I have started a rebuild of the CI image. It should be uploaded in about an hour. In the future please don't fiddle with the travis files, but change the master Dockerfile under ciimage instead.

ignatenkobrain · 2018-08-15T15:36:03Z

mesonbuild/modules/hotdoc.py

@@ -0,0 +1,317 @@
+import os
+import subprocess


[Flake8]

[F401] 'subprocess' imported but unused

thiblahute · 2018-08-16T11:55:25Z

The AppVeyor failure looks weird and is totally unrelated I believe.

MathieuDuponchelle

This otherwise looks ok to me, good job :)

MathieuDuponchelle · 2018-08-17T21:48:50Z

mesonbuild/modules/hotdoc.py

+MIN_HOTDOC_VERSION = '0.8.100'
+
+
+class HotdocTargetBuilder:


As I understand it, this builder object is simply used to hold intermediary state while processing the arguments, which is why it's separate from the actual target classes, and why you don't simply have a huge function in the module to prepare the targets. That makes sense to me, but I would suggest following that approach to its end, and simply have something like:

builder = Builder() targets = builder.make_targets()

in the generate_doc() method of the module, makes sense?

Makes sense, I started having a generic Builder that could be reused in other modules, but that experiment didn't work well, let do what you suggest, it will be cleaner :-)

MathieuDuponchelle · 2018-08-17T22:16:39Z

@thiblahute , you will also need to add documentation, and a release snippet

The module might need to add extra methods on the returned targets and thus it can return TargetHolders, we should process the held targets

thiblahute · 2018-08-18T13:41:01Z

Added documentation and made the small refactoring @MathieuDuponchelle asked for.

MathieuDuponchelle · 2018-08-18T14:48:49Z

We obviously will need @jpakkane 's approval, but this looks good to go to me now :)

jpakkane · 2018-08-18T16:20:08Z

The sources argument seems to only handle strings. Should it work with files objects as well? I would imagine people wanting to do something like:

sources = files(...)
shared_library('something', sources)
hotdoc.generate_doc(...
  c_sources : files,
  ...
  )

To only have to list the files once?

hotdoc: http://github.com/hotdoc/hotdoc/

Let's eat our own dogfood.

thiblahute · 2018-08-29T19:55:41Z

Thanks :-)

marc-h38 · 2020-02-21T22:23:51Z

Hi Thibault. It's not very intuitive to have to invoke ninja upload to merely rebuild the documentation (and not upload anything).

It's also strange that a bare ninja builds the documentation but only after a ninja clean... and without colored output?!? Weird.

I'd like to make this better but right now I'm completely lost in these inconsistencies. Care to shed some light?

PS: all the above observed with today's master branch at commit be9bff8

thiblahute · 2020-02-22T01:49:09Z

It's not very intuitive to have to invoke ninja upload to merely rebuild the documentation (and not upload anything).

ninja upload? Never heard of that! :-)

It's also strange that a bare ninja builds the documentation but only after a ninja clean... and without colored output?!? Weird.

It doesn't build the doc by default unless you specify it when you declare the target, it is build only when installing or when invoking the ninja targetname-doc.

marc-h38 · 2020-02-25T22:13:20Z

Hi Thibault, I think I figured out why we were talking across each other. I should have written that I'm very specifically referring to building the documentation of meson itself and your commit 6f72473b2457ecd4, and especially this line of yours:

ninja -C build/ upload

thiblahute · 2020-02-26T00:13:14Z

Hi Thibault, I think I figured out why we were talking across each other. I should have written that I'm very specifically referring to building the documentation of meson itself and your commit 6f72473, and especially this line of yours:
ninja -C build/ upload

Ah, I just ported previous doc there (where it was explaining how to upload the doc), you can just run ninja -C build/ in there as the hotdoc target is build_by_default: true

marc-h38 · 2020-02-26T01:03:12Z

ninja -C build works but only the first time. ninja -C build upload seems to rebuild every time; while not ideal it's much better.

$ touch markdown/*
$ ninja -C build/
ninja: Entering directory `build/'
ninja: no work to do.
$ ninja -C build/ upload
ninja: Entering directory `build/'
[0/1] Running external command upload.
WARNING: [core]: (markdown-bad-link): ...

MathieuDuponchelle · 2020-02-26T12:38:24Z

ninja -C build clean && ninja -C build, while not ideal, should do the trick. hotdoc has support for generating deps files:

  --deps-file-dest DEPS_FILE_DEST
                        Where to output the dependencies file
  --deps-file-target DEPS_FILE_TARGET
                        Name of the dependencies target

but I don't think this is used by the hotdoc module.

thiblahute · 2020-02-26T13:18:09Z

but I don't think this is used by the hotdoc module.

It does and it worked at some point, we should check why it doesn't anymore.

MathieuDuponchelle · 2020-02-26T13:19:17Z

Oh, good to know, then yeah that would be the thing to check first :)

xclaesse · 2020-02-26T14:44:08Z

@MathieuDuponchelle @thiblahute : While you're here, would you guys agree to add yourself in CODEOWNERS for the hotdoc module?

thiblahute · 2020-02-26T15:12:05Z

@MathieuDuponchelle @thiblahute : While you're here, would you guys agree to add yourself in CODEOWNERS for the hotdoc module?

I guess I am fine with that, what does it involve in practice?

xclaesse · 2020-02-26T15:19:18Z

You get requested review automatically for any PR that touch that module. More generally, you are the maintainer of that module, you take the decisions about new features, etc that gets included. I've been pushing to delegate more responsibilities in the Meson project: #6485.

thiblahute · 2020-02-26T15:48:31Z

I am fine with that.

MathieuDuponchelle · 2020-02-26T15:54:27Z

that is a good idea yes :) thib knows more about the module than I do however

marc-h38 · 2020-02-26T17:38:29Z

ninja -C build clean && ninja -C build, while not ideal

While I have admittedly no idea why it works, ninja -C build upload seems to do the same, is shorter to type and is what @koponomarenko documented in meson/docs/README.md (PR #4275 commit e53be9c - before CODEOWNERS I guess :-)

hotdoc has support for generating deps files:

Depending on the complexity this may be overkill for meson/docs/ which take less than 2 seconds to generate.

MathieuDuponchelle · 2020-02-26T17:54:36Z

While I have admittedly no idea why it works, ninja -C build upload seems to do the same

Sure, but if the goal is to simply rebuild the local copy, then there's not much point in trying to upload to a place where you probably don't have write access :D

Depending on the complexity this may be overkill for meson/docs/ which take less than 2 seconds to generate.

Right, but meson needs some trigger to make it rebuild when some dep has changed doesn't it?

marc-h38 · 2020-02-26T19:58:50Z

then there's not much point in trying to upload to a place where you probably don't have write access :D

Let me quote the very first sentence I used to start this recent discussion:

Hi Thibault. It's not very intuitive to have to invoke ninja upload to merely rebuild the documentation (and not upload anything).

Back to you:

Right, but meson needs some trigger to make it rebuild when some dep has changed doesn't it?

I meant: no, in this very particular case it could just rebuild every time because it's quick. Like ninja upload already does (how?) for some unknown reason.

MathieuDuponchelle · 2020-02-26T21:00:00Z

Let me quote the very first sentence I used to start this recent discussion:

Hi Thibault. It's not very intuitive to have to invoke ninja upload to merely rebuild the documentation (and not upload anything).

Yes? My point is that ninja upload isn't the correct command to use to rebuild the doc locally, I gave you the correct one and you replied something about conciseness, I don't really get your point now

I meant: no, in this very particular case it could just rebuild every time because it's quick. Like ninja upload already does (how?) for some unknown reason.

ninja upload presumably uses hotdoc's uploading feature, and hotdoc always rebuilds when called. Re rebuilding every time, that's hardly the desirable outcome, a well-behaved project should do nothing on the second of two successive calls to ninja -C build, I'm sure we can agree on that. Repeating what I said earlier, we should look into why the depsfile setup is broken, it's probably not very difficult to fix.

thiblahute · 2020-02-27T00:39:05Z

OK, here is the fix in meson: hotdoc/hotdoc#184

Although I add this issue with ninja 1.9 which is fixed in ninja 1.10.

marc-h38 · 2020-02-27T02:04:13Z

My point is that ninja upload isn't the correct command to use to rebuild the doc locally, I gave you the correct one and you replied something about conciseness, I don't really get your point now

Involving ninja clean; ninja every time is neither convenient nor the "correct" way to build anything.

I will try the fix, thanks Thibault.

MathieuDuponchelle · 2020-02-27T16:16:10Z

Involving ninja clean; ninja every time is neither convenient nor the "correct" way to build anything.

It's the correct way if you want to rebuild from scratch, using the upload target as a workaround clearly isn't, anyway this argument is pointless

marc-h38 · 2020-03-21T22:33:03Z

It [ninja clean] is the correct way if you want to rebuild from scratch,

Agreed, keeping in mind rebuilding from scratch is a workaround in the first place.

Unless changes are made to the meson.build or related files themselves and then ninja clean is regularly not enough - I wasted way too much time trusting ninja clean in that case so I simply stopped using it when editing meson.build. rm -rf build or git clean -fdx are then the only way. Example: https://github.com/zephyrproject-rtos/zephyr/blob/master/cmake/pristine.cmake

See also "meson + ninja wrapper script" comment here: #6434 (comment)

tl;dr: using ninja clean is always the sign something somewhere is wrong.

anyway this argument is pointless

Agreed, as in: " is bad better than very bad?"

MathieuDuponchelle · 2020-03-23T14:58:49Z

" is bad better than very bad?"

Good news is with thib's fix the behaviour is correct again so we can forget about the argument :D

eli-schwartz · 2020-03-23T17:09:04Z

Unless changes are made to the meson.build or related files themselves and then ninja clean is regularly not enough - I wasted way too much time trusting ninja clean in that case so I simply stopped using it when editing meson.build. rm -rf build or git clean -fdx are then the only way.

If meson's builtin support for reconfiguring on changes to meson.build isn't enough for you (why not???) then I would recommend using meson setup --wipe build/ instead, which should be the same as rm -rf build/ except it preserves the command line you previously used. ;)

marc-h38 · 2020-03-23T20:19:22Z

Good news is with thib's fix the behaviour is correct again

I thought I was trying the fix yesterday but realized at last the fix is in hotdoc itself and not in the meson module and that the fixed hotdoc version isn't released in pip yet :-/ Will try again another time.

If meson's builtin support for reconfiguring on changes to meson.build isn't enough for you (why not???)

It is enough... most but not all the time. When changing cross files maybe? I'm actually amazed it is enough most of the time, however my project is small enough that it's just faster to rm -rf build/ and not think about edge cases.

I would recommend using meson setup --wipe build/ instead

I tried --wipe and I'd love to use it, however it fails when build/ doesn't exist so rm -rf build is just simpler to script. Already reported in #6434 (comment)

thiblahute force-pushed the hotdoc branch from b4597d0 to d6a8535 Compare August 13, 2018 22:18

ignatenkobrain reviewed Aug 13, 2018

View reviewed changes

thiblahute force-pushed the hotdoc branch from d6a8535 to 4b2c16a Compare August 14, 2018 13:23

thiblahute force-pushed the hotdoc branch from 4b2c16a to 0f2968b Compare August 14, 2018 13:32

MathieuDuponchelle reviewed Aug 14, 2018

View reviewed changes

thiblahute force-pushed the hotdoc branch from 0f2968b to 4ea74c7 Compare August 14, 2018 15:02

thiblahute force-pushed the hotdoc branch 4 times, most recently from 705724b to 5b73a0d Compare August 15, 2018 15:35

ignatenkobrain reviewed Aug 15, 2018

View reviewed changes

thiblahute force-pushed the hotdoc branch from 5b73a0d to 3c6107f Compare August 15, 2018 16:53

MathieuDuponchelle suggested changes Aug 17, 2018

View reviewed changes

thiblahute added 2 commits August 18, 2018 09:04

interpreter: Handle 'bool, str, int' values returned by modules

a95763c

interpreter: Also process *TargetHolder returned by modules

221ac05

The module might need to add extra methods on the returned targets and thus it can return TargetHolders, we should process the held targets

thiblahute force-pushed the hotdoc branch from 3c6107f to 43a35f7 Compare August 18, 2018 12:04

MathieuDuponchelle approved these changes Aug 18, 2018

View reviewed changes

thiblahute force-pushed the hotdoc branch from 43a35f7 to 8d87478 Compare August 18, 2018 17:30

thiblahute added 3 commits August 28, 2018 18:18

modules: Add an 'hotdoc' module

378bd4d

hotdoc: http://github.com/hotdoc/hotdoc/

docs: Use meson to build documentation

6f72473

Let's eat our own dogfood.

hotdoc: Add documentation

1ea743a

thiblahute force-pushed the hotdoc branch from ebb43c1 to 1ea743a Compare August 28, 2018 21:18

jpakkane merged commit b86f2fd into mesonbuild:master Aug 29, 2018



		def initialize(interpreter):
		return HotDocModule(interpreter)

		MIN_HOTDOC_VERSION = '0.8.100'


		class HotdocTargetBuilder:

modules: Add an 'hotdoc' module #4016

modules: Add an 'hotdoc' module #4016

Conversation

thiblahute commented Aug 13, 2018

ignatenkobrain commented Aug 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thiblahute commented Aug 14, 2018

Choose a reason for hiding this comment

thiblahute Aug 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpakkane commented Aug 14, 2018

Choose a reason for hiding this comment

thiblahute commented Aug 16, 2018

MathieuDuponchelle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MathieuDuponchelle commented Aug 17, 2018

thiblahute commented Aug 18, 2018

MathieuDuponchelle commented Aug 18, 2018

jpakkane commented Aug 18, 2018

thiblahute commented Aug 29, 2018

marc-h38 commented Feb 21, 2020 • edited Loading

thiblahute commented Feb 22, 2020

marc-h38 commented Feb 25, 2020

thiblahute commented Feb 26, 2020

marc-h38 commented Feb 26, 2020

MathieuDuponchelle commented Feb 26, 2020

thiblahute commented Feb 26, 2020

MathieuDuponchelle commented Feb 26, 2020

xclaesse commented Feb 26, 2020

thiblahute commented Feb 26, 2020

xclaesse commented Feb 26, 2020

thiblahute commented Feb 26, 2020

MathieuDuponchelle commented Feb 26, 2020

marc-h38 commented Feb 26, 2020

MathieuDuponchelle commented Feb 26, 2020

marc-h38 commented Feb 26, 2020

MathieuDuponchelle commented Feb 26, 2020

thiblahute commented Feb 27, 2020

marc-h38 commented Feb 27, 2020 • edited Loading

MathieuDuponchelle commented Feb 27, 2020

marc-h38 commented Mar 21, 2020

MathieuDuponchelle commented Mar 23, 2020

eli-schwartz commented Mar 23, 2020

marc-h38 commented Mar 23, 2020

thiblahute Aug 14, 2018 •

edited

Loading

marc-h38 commented Feb 21, 2020 •

edited

Loading

marc-h38 commented Feb 27, 2020 •

edited

Loading