Re: [RFC PATCH v2 1/1] devtools: add vscode configuration generator

[Bruce Richardson <bruce.richardson@intel.com>]

On Tue, Jul 30, 2024 at 11:21:25AM +0200, Burakov, Anatoly wrote:
> On 7/29/2024 6:41 PM, Bruce Richardson wrote:
> > On Mon, Jul 29, 2024 at 06:16:48PM +0200, Burakov, Anatoly wrote:
> > > On 7/29/2024 4:30 PM, Bruce Richardson wrote:
> > > > On Mon, Jul 29, 2024 at 02:05:52PM +0100, Anatoly Burakov wrote:
> > > > > A lot of developers use Visual Studio Code as their primary IDE. This
> > > > > script generates a configuration file for VSCode that sets up basic build
> > > > > tasks, launch tasks, as well as C/C++ code analysis settings that will
> > > > > take into account compile_commands.json that is automatically generated
> > > > > by meson.
> > > > > 
> > > > > Files generated by script:
> > > > >    - .vscode/settings.json: stores variables needed by other files
> > > > >    - .vscode/tasks.json: defines build tasks
> > > > >    - .vscode/launch.json: defines launch tasks
> > > > >    - .vscode/c_cpp_properties.json: defines code analysis settings
> > > > > 
> > > > > The script uses a combination of globbing and meson file parsing to
> > > > > discover available apps, examples, and drivers, and generates a
> > > > > project-wide settings file, so that the user can later switch between
> > > > > debug/release/etc. configurations while keeping their desired apps,
> > > > > examples, and drivers, built by meson, and ensuring launch configurations
> > > > > still work correctly whatever the configuration selected.
> > > > > 
> > > > > This script uses whiptail as TUI, which is expected to be universally
> > > > > available as it is shipped by default on most major distributions.
> > > > > However, the script is also designed to be scriptable and can be run
> > > > > without user interaction, and have its configuration supplied from
> > > > > command-line arguments.
> > > > > 
> > > > > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > > > > ---
> > > > > 
> > > > Just was trying this out, nice script, thanks.
> > > 
> > > Thanks for the feedback! Comments below.
> > > 

More comments inline below, but summarising after the fact here.

 Still not entirely sure what way is best for all this so please take all
current and previous suggestions with a pinch of salt. Based off what you
suggest and the ongoing discuss my current thinking is:

* +1 to split the vscode config generation from the TUI. Both are also
  targetting different end-users - the TUI is for everyone looking to build
  DPDK, both devs and users, while the vscode config is for developers only.
* Let's ignore the multi-build-directory setup for now, if it makes it more
  complex for the simple cases of one build-dir.
* I think we should investigate having the vscode config generated from
  meson rather than the other way around.

See also below.

/Bruce

> > > > 
> > > > Initial thoughts concerning the build directory:
> > > > - the script doesn't actually create the build directory, so there is no
> > > >     guarantee that the build directory created will have the same parameters
> > > >     as that specified in the script run. I'd suggest in the case where the
> > > >     user runs the script and specifies build settings, that the build
> > > >     directory is then configured using those settings.
> > > 
> > > I'm not sure I follow.
> > > 
> > > The script creates a command for VSCode to create a build directory using
> > > configuration the user has supplied at script's run time. The directory is
> > > not created by the script, that is the job of meson build system. This
> > > script is merely codifying commands for meson to do that, with the
> > > expectation that the user is familiar with VSCode workflow and will go
> > > straight to build commands anyway, and will pick one of them. Are you
> > > suggesting running `meson setup` right after?
> > > 
> > 
> > Yes, that is what I was thinking, based on the assumption that running
> > "meson setup" should be a once-only task. I suppose overall it's a
> > different workflow to what you have - where you run meson setup repeatedly
> > each time you change a build type. My thinking for the approach to take
> > here is that your script firstly asks for a build directory, and then:
> > * if build-dir exists, pull settings you need from there, such as build
> >    type and the apps being built.
> > * if not existing, ask for config settings as you do now, and then run
> >    meson setup to create the build dir.
> 
> I guess the disconnect comes from the fact that I treat meson build
> directories as transient and disposable and never hesitate to wipe them and
> start over, whereas you seem to favor persistence. Since 99% of the time I'm
> using heavily reduced builds anyway (e.g. one app, one driver), repeatedly
> doing meson setup doesn't really hinder me in any way, but I suppose it
> would if I had more meaty builds.
> 

It mainly just seems inefficient to me. Ninja does a lot of processing to
minimise the work done whenever you make a configuration change, and you
bypass all that by nuking the directory from orbit (only way to be sure!)
and then creating a new one!

The other main downside of it (to my mind), is that the tracking of
settings for the build needs to be in vscode. I'd prefer meson itself to
be the "one source of truth" and vscode to be tweaking that, rather than
tracking everything itself.

> I think we also have a slightly different view of what this script should be
> - I envisioned a "one-stop-shop" for "open freshly cloned DPDK directory,
> run one script and off you go" (which stems from the fact that I heavily
> rely on Git Worktrees [1] in my workflow, so having an untouched source
> directory without any configuration in it is something I am constantly faced
> with), while you seem to favor picking up existing meson build and building
> a VSCode configuration around it.
> 
> I can see point in this, and I guess I actually unwittingly rolled two
> scripts into one - a TUI meson frontend, and a VSCode configuration
> generator. Perhaps it should very well be two scripts, not one? Because
> honestly, having something like what I built for TUI (the meson
> configuration frontend) is something I missed a lot and something I always
> wished our long-gone `setup.sh` script had, but didn't, and now with meson
> it's much simpler to do but we still don't have anything like that. Maybe
> this is our opportunity to provide a quicker "quick start" script, one that
> I could run and configure meson build without having to type anything. WDYT?
> 

Splitting it into two makes sense to me, yes, since as you point out there
are really two separate jobs involved here that one may want to roll with
separately.

> > 
> > Thereafter, the source for all build settings is not in vscode, but in
> > meson, and you just use "meson configure" from vscode to tweak whatever
> > needs tweaking, without affecting any other settings. Since you don't
> > affect any other settings there is no need to record what they should be.
> 
> Why would I be using meson configure from vscode then? I mean, the whole
> notion of having tasks in VSCode is so that I define a few common
> configurations and never touch them until I'm working on something else. If
> I have to type in configure commands anyway (because I would have to, to
> adjust configuration in meson), I might as well do so using terminal, and
> avoid dealing with meson from VSCode entirely?
> 
> (technically, there's a way to read arguments from a popup window - I
> suppose we could have this step recorded as a VSCode task)
> 

The reason I was thinking about this is that you don't want to expose
dozens of tasks in vscode for tweaking every possible meson setting. Sure,
have build and run tasks for the common options, but for "advanced use"
where the user wants to tweak a build setting, let them just do it from
commandline without having to re-run a config script to adjust the
settings.

<Snip>

> > > > 
> > > > - Finally, and semi-related, this script assumes that the user does
> > > >     everything in a single build directory. Just something to consider, but
> > > >     my own workflow till now has tended to keep multiple build directories
> > > >     around, generally a "build" directory, which is either release or
> > > >     debugoptimized type, and a separate "build-debug" directory + occasionally
> > > >     others for build testing. When doing incremental builds, the time taken to
> > > >     do two builds following a change is a lot less noticable than the time taken
> > > >     for periodic switches of a single build directory between debug and release
> > > >     mode.
> > > 
> > > The problem with that approach is the launch tasks, because a launch task
> > > can only ever point to one executable, so if you have multiple build
> > > directories, you'll have to have multiple launch tasks per app/example. I
> > > guess we can either tag them (e.g. "Launch dpdk-testpmd [debug]", "Launch
> > > dpdk-testpmd [asan]" etc.), or use some kind of indirection to "select
> > > active build configuration" (e.g. have one launch task but overwrite
> > > ${config:BUILDDIR} after request for configuration, so that launch tasks
> > > would pick up actual executable path at run time from settings). I would
> > > prefer the latter to be honest, as it's much easier to drop a script into
> > > ./vscode and run it together with "configure" command to switch between
> > > different build/launch configurations. What do you think?
> > > 
> > 
> > I think I'd prefer the former actually - to have explicit tasks always
> > listed for debug and release builds.
> > Not a big deal for me either way, I'll just hack in the extra tasks as I
> > need them, so it's a low-priority support item for me.
> 
> There's another issue though: code analysis. If you have multiple build
> directories, your C++ analysis settings (the .vscode/c_cpp_properties.json
> file) can only ever use one specific compile_commands.json from a specific
> build directory. I think if we were to support having multiple build dirs,
> we would *have to* implement something like "switch active configuration"
> thing anyway.
> 

Strictly speaking, yes. However, in my experience using eclipse as an IDE
in the past it doesn't matter that much which or how many build directories
are analysed. However, vscode may well be different in this regard.

Since I don't ever envisage myself doing everything always through vscode,
I'm happy enough with vscode managing a single build directory, and I can
manually worry about a second build directory myself.  Maybe let's park the
multi-build-dir stuff for now, unless others feel that it's something they
need.

> Whether we add tagged launch tasks is honestly a problem to be solved,
> because on the one hand we want them to be generated automatically (ideally
> after configure step), and on the other we also want to not interfere with
> any custom configuration the user has already added (such as e.g. command
> line arguments to existing launch tasks). We'll probably have to do config
> file parsing and updating the configuration, rather than regenerating it
> each time. Python's JSON also doesn't support comments, so for any comments
> that were added to configurations, we'd either lose them, or find a way to
> reinsert them post-generation.
> 

Have you considered generating the launch tasks from a script launched from
meson itself? Any time the configuration is changed, meson will re-run at
the next build and that can trigger re-generation of whatever vscode config
you need, including launch tasks for all the binaries. This would be
another advantage of splitting the script into two - one should look to make
the vscode-settings generation script usable from meson.

<snip>

> It sounds like this would really be something that a setup script would do
> better than a VSCode config generator.
> 
> So, assuming we want to move setup steps to another script and concentrate
> on VSCode configuration exclusively, my thinking of how it would work is as
> follows:
> 
> 1) Assume we want multiple build directories, suggest automatically picking
> them up from source directory but support specifying one or more from
> command line arguments (or TUI, although I suspect if setup moves to a
> separate script, there's very little need for TUI in VSCode config generator
> - it should be a mostly mechanical process at that point)
> 

I'm probably an outlier, so lets not over-design things for the
multi-build-directory case.

If we think about generating the vscode config from a meson run (via a
"generate-vscode-config" setting or something), that may switch the way in
which things are actually being done. In that case, a build directory would
register itself with the vscode config - creating a new one if not already
present.

> 2) Now that we track multiple build directories, we can store them in a YAML
> file or something (e.g. .vscode/.dpdk_builds.yaml), and create tasks to
> switch between them as "active" to support e.g. different code analysis
> settings and stuff like that
> 

Initially, maybe don't add this. For first draft supporting
multi-directories, I'd start by adding prefixed duplicate tasks for each
directory registered.

> 3) All build tasks can work off "active configuration" which means we don't
> need multiple compile tasks, but for launch tasks we may need different ones
> because different build dirs may have different launch tasks
> 

Again, I'd just add tasks rather than bothering with active configs.

> Let's assume user just ran `meson configure` and changed something about one
> of their configurations. What do you think should happen next? I mean, if
> they added/removed an app/example to the build, we can detect that and
> auto-generate (or remove) a launch task, for example? Or do you think it
> should be a manual step, e.g. user should explicitly request
> regenerating/updating launch tasks? Maybe there should be an --update flag,
> to indicate that we're not creating new configurations but merely refreshing
> existing ones?

See above, this is a case where having the vscode config script callable
from meson would be perfect.

> 
> [1] https://git-scm.com/docs/git-worktree
>
Thanks for the link - seems to be automating a setup which I've been
approximately doing manually for some years now! :-)