[cmake-developers] daemon-mode: Project structure

[Stephen Kelly]

On 06/10/2016 11:35 PM, Tobias Hunger wrote:
>
> > Part of the design of the daemon is that messages that it sends can be
> > 'spontaneous' - it watches for filesystem changes and can tell
> clients to
> > re-read the buildsystem information.
>
> That is currently not done or needed, but can be added.
>

I think this is essential to the first version of the protocol.

The daemon is a long-running process, so it needs to be able to report
about changes it is (designed to be) aware of.

The central goal of the daemon is to make it easier to implement good
support in existing tools and in new tools. In your branch you have a
reset command in the protocol. I think that should be dropped because
the daemon protocol should be designed such that it is never needed.

> > Additionally, it appears to be
> > redundant if you have a 'cookie'?
>
> No, cookies are to attach user data to requests and that is entirely
> orthogonal to what inReplyTo does.
>

Ok. Then I don't understand why clients need inReplyTo. I don't
understand why it is not redundant. Clients know what cookie they
specified in a request and they get the same cookie back, so they can
check their cookie jar and get the same information as inReplyTo, right?

> > It is important that the daemon
> > does not create a new claim of truth in this regard - the structure
> from the
> > daemon should be the same as the structure generated for those ide
> projects.
>
> Why?
>
> I want the structure that *CMake* sees. What some unrelated program
> generates out of that structure is irrelevant.
>

What CMake sees is what it generates for IDE buildsystems.

Try taking a buildsystem of multiple directories and targets and putting
project() commands at various positions - before/after add_subdirectory
calls, in between targets, top of the file etc, and see what CMake does
there.

You might have an argument for *changing* what CMake does there, but you
would need to state such a change explicitly. (and to do that you need
to be aware of it in the first place)

This is part of the design work for the daemon because the behavior of
the daemon should not be accidental in any way. While protocol
versioning will make it possible to introduce new versions of the
protocol, old versions of the protocol will need to remain, and that can
affect the C++ code of CMake in bad ways, particularly if what is
exposed in the protocol is 'wrong' in some way. Just look at CMake
policies - the first ones were introduced a decade ago and there is no
plan for removing them.

I emphasise the above just to make the points that:

* Designing a protocol like this is hard (and not fast)
* We should introduce something minimal, avoiding redundancy, because
removing things is hard
* The protocol should possibly expose a way to set policies so that they
can be used to deprecate behaviors in a compatible way, as is done
elsewhere in CMake
* The version of the protocol should correspond to the version of CMake
it appears in (Just like how QDataStream works). Currently you're
versioning the protocol as '0.1' instead of the CMake version. Look at
the mess of QML versions, and let's not reproduce that here :)

 http://thread.gmane.org/gmane.comp.lib.qt.devel/23246

> <snip>
>
> > > Is this the information you need for IDE integration?
> >
> > You might be packing too much into one protocol verb. Try this on
> LLVM and
> > VTK for example to see how much data it is. Perhaps then compare
> with the
> > gradual approach in my branch.
>
> CMake itself is way below 100KiB.
>
> A typical developer machine should be able to handle several 10MiB of
> JSON data just fine. So I do not expect a problem there.
>

Then we can easily verify that and the impact (latency difference etc)
with larger projects like the ones I mentioned.

I don't know what's better but we should be able to quantify the impact
of the choices made about what to return from particular protocol queries.

> > In my branch I have
> >
> > * `buildsystem` - Get the targets (name, type, project) and backtraces
> > * `target_info` - Get buildsystem properties for a target
> > * `file_info` - Get buildsystem properties for a file in a target
>
> I know. I did not like that, so I did it differently.
>
> Basically I do want the really important information in one go instead
> of requesting a little bit, iterate over the data, request some more, ...
>

Clients don't necessarily need all of it just to start up and show
something on start. Maybe QtCreator does, I don't know.

> In the end that is a lots of needless round trips and you send more
> data over the wire, too, with all the protocol overhead.
>

That's something that should be quantifyable.

If it makes no difference to compute and send it all, even on large
projects then maybe that's fine.

I think figuring this stuff out is part of the work of designing the
protocol.

> > Getting the backtraces (instead of the location) for targets is
> important -
>
> I am aware of the importance of backtraces, but I do not see any use
> case where this information needs to be available at the point the
> project structure is requested.
>

It should also be possible to quantify the impact of that. How much
extra data is it? How much latency is imposed if every time I click on a
target in a treeview there is a round-trip to the server before the
application even knows what should be shown in the editor next as a
result of the click? How much simpler is the client code if they already
have that information associated with the treeview node?

> An IDE can for example request backtraces for a target when/if needed
> and does not need it right away to set up a project tree and code model.
>

What about a clickable project tree?

There are several different concerns here. You proposal might be the
right answer, but my central point is that work needs to be done to
analyze the needs and be explicit about the choices and trade-offs made.

> A tool to run a static analyzer over all C++ files in a project will
> not need backtraces ever.
>

A tool which *only* runs a windowless static analyzer on C++ files of a
buildsystem without showing the buildsystem will probably not use the
daemon. Depending on how the static analyzer works it would be likely
that a different CMake feature would be more suitable.

> The biggest part of your mail is about the history of the patch set
> and how to collaborate further.
>
> I did that to make review simpler for *other* reviewers.
>

It makes sense to be able to compare your work with what came before it.
There is a large design space here and it should be understood before
merging a protocol to master.

Renaming the class and renaming member variables (m_Foo is not
conventional in CMake code) makes that harder.

> I had not expected you to chime in at all. Don't get me wrong: I
> appreciate you experience, your patience and your reviews! I just had
> not expected it. The impression I got talking to you before is that
> you do not have the resources to push this functionality forward at
> this time. In fact that is the *only* reason I stepped up for this task.
>

Yes, I will provide guidance on this topic where I can.

> This is why I wonder how working through your branch is supposed to
> function within the resource limits on your side?
>

I still have knowledge to share, even if I'm not putting time into CMake
development now :).

> I do not want for this work to vanish into some obscure github branch
> somewhere for weeks and month -- as it did so far. The CMake daemon is
> too cool a feature for that!
>

It didn't really vanish. It's a fairly well-known repo, blog and video.

No one worked on the topic for weeks or months. That is changing now,
but the work to be done is still the same as it was in January. It's
lots of design and analysis, and refactoring of CMake itself. The
patches extracted from your branch in the last weeks are great progress
for that, as is the wide-ranging refactoring from Daniel as he has more
experience with the core of CMake.

I don't think 'time to master' is an appropriate concern for a feature
which has not received enough attention yet. This stuff is complex and I
expect that doing it right would take more 'weeks and months'.

My central point is that the design space for the protocol needs to be
explored and understood so that:

* What it exposes is correct from the CMake POV (what is 'correct' might
have to be changed *first*, as you suggest might be necessary with the
project parts).
* It can be used by QtCreator
* It can be used by other tools
* It can be extended with some of the kinds of features I showcased

I don't think the protocol should be merged before that kind of design
work is done, but ultimately it's indeed not up to me.

Maybe we should talk about these issues over a beer instead :).

Thanks,

Steve.