diff mbox series

vscode: improve settings for new yocto plugin

Message ID 20240209203029.3801065-1-adrian.freihofer@siemens.com
State New
Headers show
Series vscode: improve settings for new yocto plugin | expand

Commit Message

Adrian Freihofer Feb. 9, 2024, 8:30 p.m. UTC
There is a new official bitbake plugin:
https://marketplace.visualstudio.com/items?itemName=yocto-project.yocto-bitbake

This plugin automatically updates the .vscode/settings.json file.

Having the settings.json file in git and a plugin which modifies this
file is not ideal. It can lead to anoying situations especially when
working with git. For example a git stash reverts the settings which are
automatically applied by the plugin. While git stashed the settings.json
file the plugin immediately changes the file again and tries to run
bitbake based on the newly generated settings. When git does a stash pop
the restored settings.json file conflicts with the new settigns.json
file which has been generated while the git stash took place.

Removing the settings.json from git would lead to other issues as
already described in the commit message of 5ff688fe29. Until VSCode or
the plugin supports multiple config files like bitbake does with the
site.conf and the local.conf files, there are probably some use cases
and workflows where the plugin needs to be disabled.

This commit aligns the default settings.json file with the default
settings.json which gets generated by the plugin. This is a workaround
for the issue described above. But it works only for the default build
configuration with one build folder named "build".

Discussion is here:
https://github.com/yoctoproject/vscode-bitbake/issues/95
---
 .gitignore            |  4 ++--
 .vscode/settings.json | 28 +++++++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 3 deletions(-)

Comments

Richard Purdie Feb. 9, 2024, 11:43 p.m. UTC | #1
On Fri, 2024-02-09 at 21:30 +0100, Adrian Freihofer wrote:
> There is a new official bitbake plugin:
> https://marketplace.visualstudio.com/items?itemName=yocto-project.yocto-bitbake
> 
> This plugin automatically updates the .vscode/settings.json file.
> 
> Having the settings.json file in git and a plugin which modifies this
> file is not ideal. It can lead to anoying situations especially when
> working with git. For example a git stash reverts the settings which are
> automatically applied by the plugin. While git stashed the settings.json
> file the plugin immediately changes the file again and tries to run
> bitbake based on the newly generated settings. When git does a stash pop
> the restored settings.json file conflicts with the new settigns.json
> file which has been generated while the git stash took place.
> 
> Removing the settings.json from git would lead to other issues as
> already described in the commit message of 5ff688fe29. Until VSCode or
> the plugin supports multiple config files like bitbake does with the
> site.conf and the local.conf files, there are probably some use cases
> and workflows where the plugin needs to be disabled.
> 
> This commit aligns the default settings.json file with the default
> settings.json which gets generated by the plugin. This is a workaround
> for the issue described above. But it works only for the default build
> configuration with one build folder named "build".
> 
> Discussion is here:
> https://github.com/yoctoproject/vscode-bitbake/issues/95
> ---
>  .gitignore            |  4 ++--
>  .vscode/settings.json | 28 +++++++++++++++++++++++++++-
>  2 files changed, 29 insertions(+), 3 deletions(-)

I am very worried about this as we can't mandate a layout. There are
many people who use different locations for bitbake and others who use
different directories for builds. Even our own test runs do change the
build directories.

I don't think we can solve this like this unfortunately.

Can we inject something into build directories when we create them?

Cheers,

Richard
Adrian Freihofer Feb. 10, 2024, 1:42 p.m. UTC | #2
On Fri, 2024-02-09 at 23:43 +0000, Richard Purdie wrote:
> On Fri, 2024-02-09 at 21:30 +0100, Adrian Freihofer wrote:
> > There is a new official bitbake plugin:
> > https://marketplace.visualstudio.com/items?itemName=yocto-project.yocto-bitbake
> > 
> > This plugin automatically updates the .vscode/settings.json file.
> > 
> > Having the settings.json file in git and a plugin which modifies
> > this
> > file is not ideal. It can lead to anoying situations especially
> > when
> > working with git. For example a git stash reverts the settings
> > which are
> > automatically applied by the plugin. While git stashed the
> > settings.json
> > file the plugin immediately changes the file again and tries to run
> > bitbake based on the newly generated settings. When git does a
> > stash pop
> > the restored settings.json file conflicts with the new
> > settigns.json
> > file which has been generated while the git stash took place.
> > 
> > Removing the settings.json from git would lead to other issues as
> > already described in the commit message of 5ff688fe29. Until VSCode
> > or
> > the plugin supports multiple config files like bitbake does with
> > the
> > site.conf and the local.conf files, there are probably some use
> > cases
> > and workflows where the plugin needs to be disabled.
> > 
> > This commit aligns the default settings.json file with the default
> > settings.json which gets generated by the plugin. This is a
> > workaround
> > for the issue described above. But it works only for the default
> > build
> > configuration with one build folder named "build".
> > 
> > Discussion is here:
> > https://github.com/yoctoproject/vscode-bitbake/issues/95
> > ---
> >  .gitignore            |  4 ++--
> >  .vscode/settings.json | 28 +++++++++++++++++++++++++++-
> >  2 files changed, 29 insertions(+), 3 deletions(-)
> 
> I am very worried about this as we can't mandate a layout. There are
> many people who use different locations for bitbake and others who
> use
> different directories for builds. Even our own test runs do change
> the
> build directories.
> 
> I don't think we can solve this like this unfortunately.

Unfortunately, you are right. I agree that we need a better and most
likely a completely different solution.
The intention of this patch was to get the plugin working as a short
term solution for the default Yocto setup and gain more time to find
and develop the right solution. But I agree, we should skip this step
and find a future-proof solution now.

> 
> Can we inject something into build directories when we create them?

Yes, that's probably the way to go. I will send a better patch:

- Remove the .vscode folder from git
  (which works without any issues with the old plugin)
- Change the .gitignore to ignore the complete .vscode folder
- New script scripts/contrib/oe-init-build-vscode which generates the
section which is currently in git and which is very important to be
present before VSCode gets started.
- Add the following to scripts/oe-buildenv-internal
  command -v code > /dev/null && ./scripts/contrib/oe-init-build-vscode

The script will not alter the settings.json if not needed to prevent
from race conditions when VSCode will do the same.


Alternative ideas:
- Use a BUILDDIR that is not in the Git tree. This is difficult because
the opposite has always been the standard.
- Improve VSCode. This would require splitting the settings.json into
different files, some of which can be in Git and some not. This is
difficult to achieve as discussions have been going on for a long time.

Thank you and regards,
Adrian


> 
> Cheers,
> 
> Richard
Adrian Freihofer Feb. 11, 2024, 3:24 p.m. UTC | #3
On Fri, 2024-02-09 at 23:43 +0000, Richard Purdie wrote:
> On Fri, 2024-02-09 at 21:30 +0100, Adrian Freihofer wrote:
> > There is a new official bitbake plugin:
> > https://marketplace.visualstudio.com/items?itemName=yocto-project.yocto-bitbake
> > 
> > This plugin automatically updates the .vscode/settings.json file.
> > 
> > Having the settings.json file in git and a plugin which modifies
> > this
> > file is not ideal. It can lead to anoying situations especially
> > when
> > working with git. For example a git stash reverts the settings
> > which are
> > automatically applied by the plugin. While git stashed the
> > settings.json
> > file the plugin immediately changes the file again and tries to run
> > bitbake based on the newly generated settings. When git does a
> > stash pop
> > the restored settings.json file conflicts with the new
> > settigns.json
> > file which has been generated while the git stash took place.
> > 
> > Removing the settings.json from git would lead to other issues as
> > already described in the commit message of 5ff688fe29. Until VSCode
> > or
> > the plugin supports multiple config files like bitbake does with
> > the
> > site.conf and the local.conf files, there are probably some use
> > cases
> > and workflows where the plugin needs to be disabled.
> > 
> > This commit aligns the default settings.json file with the default
> > settings.json which gets generated by the plugin. This is a
> > workaround
> > for the issue described above. But it works only for the default
> > build
> > configuration with one build folder named "build".
> > 
> > Discussion is here:
> > https://github.com/yoctoproject/vscode-bitbake/issues/95
> > ---
> >  .gitignore            |  4 ++--
> >  .vscode/settings.json | 28 +++++++++++++++++++++++++++-
> >  2 files changed, 29 insertions(+), 3 deletions(-)
> 
> I am very worried about this as we can't mandate a layout. There are
> many people who use different locations for bitbake and others who
> use
> different directories for builds. Even our own test runs do change
> the
> build directories.
> 
> I don't think we can solve this like this unfortunately.
> 
> Can we inject something into build directories when we create them?
> 

Here are some patches:
https://git.yoctoproject.org/poky-contrib/log/?h=adrianf/vscode-yocto-plugin

I will send them as V2 after I have tested them some more. (There are
already oe-selftest)

@Enguerrand your review would be appreciated.

Regards,
Adrian

> Cheers,
> 
> Richard
Enguerrand de Ribaucourt Feb. 12, 2024, 9:42 a.m. UTC | #4
On 11/02/2024 16:24, adrian.freihofer@gmail.com wrote:
> On Fri, 2024-02-09 at 23:43 +0000, Richard Purdie wrote:
>> On Fri, 2024-02-09 at 21:30 +0100, Adrian Freihofer wrote:
>>> There is a new official bitbake plugin:
>>> https://marketplace.visualstudio.com/items?itemName=yocto-project.yocto-bitbake
>>>
>>> This plugin automatically updates the .vscode/settings.json file.
>>>
>>> Having the settings.json file in git and a plugin which modifies
>>> this
>>> file is not ideal. It can lead to anoying situations especially
>>> when
>>> working with git. For example a git stash reverts the settings
>>> which are
>>> automatically applied by the plugin. While git stashed the
>>> settings.json
>>> file the plugin immediately changes the file again and tries to run
>>> bitbake based on the newly generated settings. When git does a
>>> stash pop
>>> the restored settings.json file conflicts with the new
>>> settigns.json
>>> file which has been generated while the git stash took place.
>>>
>>> Removing the settings.json from git would lead to other issues as
>>> already described in the commit message of 5ff688fe29. Until VSCode
>>> or
>>> the plugin supports multiple config files like bitbake does with
>>> the
>>> site.conf and the local.conf files, there are probably some use
>>> cases
>>> and workflows where the plugin needs to be disabled.
>>>
>>> This commit aligns the default settings.json file with the default
>>> settings.json which gets generated by the plugin. This is a
>>> workaround
>>> for the issue described above. But it works only for the default
>>> build
>>> configuration with one build folder named "build".
>>>
>>> Discussion is here:
>>> https://github.com/yoctoproject/vscode-bitbake/issues/95
>>> ---
>>>   .gitignore            |  4 ++--
>>>   .vscode/settings.json | 28 +++++++++++++++++++++++++++-
>>>   2 files changed, 29 insertions(+), 3 deletions(-)
>>
>> I am very worried about this as we can't mandate a layout. There are
>> many people who use different locations for bitbake and others who
>> use
>> different directories for builds. Even our own test runs do change
>> the
>> build directories.
>>
>> I don't think we can solve this like this unfortunately.
>>
>> Can we inject something into build directories when we create them?
>>My opinion as a maintainer of the VSCode extension:

The extension's users have wildly varying setups for running bitbake. 
They have divers sources and build directories layouts. They can open 
each layer in a VSCode workspace separately, while other open a parent 
directory with everything in it. Some users will use wrapper scripts 
like kas, or custom crops docker containers. There is currently no 
initiative to automatically provide initial configurations for the 
extension for these reasons.

I'm not sure to understand the application of the poky/.vscode 
configuration now. I used to see it as a way to quickly demo the VSCode 
functionalities for poky developers or beginners who have just the poky 
layer. The new patch which updated poky/.vscode was properly doing that 
IMO. A new user who downloads poky to get started with Yocto will 
directly get a .vscode to play around with the various machines and 
recipes in poky.

The majority of users who are using multiple layers and all the various 
configurations I mentioned will have to configure their own .vscode but 
I don't think they will be mislead into using poky's example 
configuration. From the user feedback we've had with the extension, I've 
never heard of users opening poky as their workspace, since most of them 
should not be editing poky in the first place.
> 
> Here are some patches:
> https://git.yoctoproject.org/poky-contrib/log/?h=adrianf/vscode-yocto-plugin
> 
> I will send them as V2 after I have tested them some more. (There are
> already oe-selftest)
> 
> @Enguerrand your review would be appreciated.
> 
> Regards,
> Adrian
> 
>> Cheers,
>>
>> Richard
> 

Thanks!
Enguerrand de Ribaucourt
Adrian Freihofer Feb. 12, 2024, 3:48 p.m. UTC | #5
Hi Enguerrand
> 
> The extension's users have wildly varying setups for running bitbake.
> They have divers sources and build directories layouts. They can open
> each layer in a VSCode workspace separately, while other open a
> parent 
> directory with everything in it. Some users will use wrapper scripts 
> like kas, or custom crops docker containers. There is currently no 
> initiative to automatically provide initial configurations for the 
> extension for these reasons.

But all users who have the build folder in the scope of VSCode have the
same need for a settings.json that prevents VSCode from indexing the
build folder and configures the yocto-bitbake plugin correctly.

> 
> I'm not sure to understand the application of the poky/.vscode 
> configuration now. I used to see it as a way to quickly demo the
> VSCode 
> functionalities for poky developers or beginners who have just the
> poky 
> layer. The new patch which updated poky/.vscode was properly doing
> that 
> IMO. A new user who downloads poky to get started with Yocto will 
> directly get a .vscode to play around with the various machines and 
> recipes in poky.

Since the plugin changes the settings.json file, the settings.json file
cannot be in Git. The problem starts at the latest when working with
git. git and VSCode fight against each other in a very problematic way.
I was not able to solve this while the plugin was still active.

> 
> The majority of users who are using multiple layers and all the
> various 
> configurations I mentioned will have to configure their own .vscode
> but 
> I don't think they will be mislead into using poky's example 
> configuration. 

The quick start guide from
https://docs.yoctoproject.org/brief-yoctoprojectqs/index.html#building-your-image
recommends
  git clone git://git.yoctoproject.org/poky
  cd poky
  source oe-init-build-env

I think just starting code like "code ." from poky or "code .." from
the build folder is the next logical step which should just work. The
Yocto/OE reference setup should also be the reference setup for the
Yocto-bitbake plugin.


> From the user feedback we've had with the extension, I've 
> never heard of users opening poky as their workspace, since most of
> them should not be editing poky in the first place.

All users who work on poky itself need to modify poky. I think
developing Yocto/OE is a viable use case for Yocto/OE.

But also all the other uses cases I'm aware of use one VSCode instance
to edit or browse all the layers in a very similar way like the quick
start suggest that.



Anyway, having the settings.json in git does no longer work. Otherwise
working with git in a layer directory is impossible.
But leaving the user without any kind of configuration leads to a user
experience like this:

  git clone git://git.yoctoproject.org/poky
  cd poky
  source oe-init-build-env
  bitbake core-image-minimal
  cd ..
  code .

- VSCode starts indexing the build folder at full CPU speed until an
OOM occurs.
- The default configuration uses the wrong build folder and other
invalid settings. The plugin is not usable without configuring many
details.

With my patches all the issues would be addressed. The plugin just
starts showing its real potential with the default configuration.

It also works for more complicated setups. I'm using it for example
with these lines in a oe-init-build-env from a custom layer:

...
if command -v code > /dev/null; then
    OEINITDIR=$(dirname "$THIS_SCRIPT")
    OEINITDIR=$(readlink -f "$OEINITDIR")
    OEINITDIR="$OEINITDIR" \
    VSCODE_EXCLUDE_DIRS="**/a-big-folder/**" \
    "$OEROOT"/scripts/contrib/oe-init-build-vscode.py || {
        unset OEROOT
        return 1
    }
fi
...


Thank you and best regards,
Adrian


> > 
> > Here are some patches:
> > https://git.yoctoproject.org/poky-contrib/log/?h=adrianf/vscode-yocto-plugin
> > 
> > I will send them as V2 after I have tested them some more. (There
> > are
> > already oe-selftest)
> > 
> > @Enguerrand your review would be appreciated.
> > 
> > Regards,
> > Adrian
> > 
> > > Cheers,
> > > 
> > > Richard
> > 
> 
> Thanks!
> Enguerrand de Ribaucourt
Enguerrand de Ribaucourt Feb. 12, 2024, 4:58 p.m. UTC | #6
On 12/02/2024 16:48, adrian.freihofer@gmail.com wrote:
> Hi Enguerrand
>>
>> The extension's users have wildly varying setups for running bitbake.
>> They have divers sources and build directories layouts. They can open
>> each layer in a VSCode workspace separately, while other open a
>> parent
>> directory with everything in it. Some users will use wrapper scripts
>> like kas, or custom crops docker containers. There is currently no
>> initiative to automatically provide initial configurations for the
>> extension for these reasons.
> 
> But all users who have the build folder in the scope of VSCode have the
> same need for a settings.json that prevents VSCode from indexing the
> build folder and configures the yocto-bitbake plugin correctly.
> 
>>
>> I'm not sure to understand the application of the poky/.vscode
>> configuration now. I used to see it as a way to quickly demo the
>> VSCode
>> functionalities for poky developers or beginners who have just the
>> poky
>> layer. The new patch which updated poky/.vscode was properly doing
>> that
>> IMO. A new user who downloads poky to get started with Yocto will
>> directly get a .vscode to play around with the various machines and
>> recipes in poky.
> 
> Since the plugin changes the settings.json file, the settings.json file
> cannot be in Git. The problem starts at the latest when working with
> git. git and VSCode fight against each other in a very problematic way.
> I was not able to solve this while the plugin was still active.
The plugin does modify .vscode/settings.json but it tries to use 
relative paths and settings that can be versioned. It should put back 
the file as it was once it's done. I haven't produced a workflow on my 
end that led to git conflicts or errors when commiting or rebasing. If 
you happen to know a reproducible workflow that does produce such 
errors, I'd be very interested in trying to fix them!

Untill then I currently don't understand the limitation of versioning 
the new settings.json you proposed some days ago.
> 
>>
>> The majority of users who are using multiple layers and all the
>> various
>> configurations I mentioned will have to configure their own .vscode
>> but
>> I don't think they will be mislead into using poky's example
>> configuration.
> 
> The quick start guide from
> https://docs.yoctoproject.org/brief-yoctoprojectqs/index.html#building-your-image
> recommends
>    git clone git://git.yoctoproject.org/poky
>    cd poky
>    source oe-init-build-env
> 
> I think just starting code like "code ." from poky or "code .." from
> the build folder is the next logical step which should just work. The
> Yocto/OE reference setup should also be the reference setup for the
> Yocto-bitbake plugin.
Before discussing what the VSCode quick start should be, here's a 
summary the current one:

The documentation for setting up the extension is hosted on it's 
marketplace frontpage. It explains how to point to an oe-init-build-env 
script, and how to prevent CPU overload by excluding the build folder. 
There are now links to it across the Yocto reference manuals in several 
places, next to Crops and Toaster: 
https://github.com/yoctoproject/vscode-bitbake/blob/staging/client/README.md#configuration

Currently the poky quick start guide is IDE agnostic. Users using VSCode 
will stumble upon the link across the Yocto docs, which provides the 
configuration guide. If they install the plugin directly, they'll also 
be prompted with the configuration guide several times. There's 
currently no mention of the poky/.vscode example configuration and we 
rely on the extension's documentation.

Similarly, the plugin guide is agnostic to the repositories setup tool 
(repo, kas, git submodules, oe-layer-setup), and build environments 
(oe-init-build-env, kas, crops, cqfd). We support all of them, the 
documentation explains how to align with them, but none is official or 
has automated setup tools.

 From what I understand, your script extends the previous setup for the 
specific case of `git clone poky` + `oe-init-build-env`. Currently this 
poky quick start, and associated config is not documented and assumes 
the user guesses where to open VSCode on their own.

The larger underlying question is, do we need to improve the quick start 
experience for VSCode users? Should we integrate the example poky VSCode 
configuration in the current documentation flow? Should VSCode be a 
reference IDE in the poky quick start guide?
> 
> 
>>  From the user feedback we've had with the extension, I've
>> never heard of users opening poky as their workspace, since most of
>> them should not be editing poky in the first place.
> 
> All users who work on poky itself need to modify poky. I think
> developing Yocto/OE is a viable use case for Yocto/OE.
> 
> But also all the other uses cases I'm aware of use one VSCode instance
> to edit or browse all the layers in a very similar way like the quick
> start suggest that.
> 
> 
> 
> Anyway, having the settings.json in git does no longer work. Otherwise
> working with git in a layer directory is impossible.
> But leaving the user without any kind of configuration leads to a user
> experience like this:
> 
>    git clone git://git.yoctoproject.org/poky
>    cd poky
>    source oe-init-build-env
>    bitbake core-image-minimal
>    cd ..
>    code .
> 
> - VSCode starts indexing the build folder at full CPU speed until an
> OOM occurs.
> - The default configuration uses the wrong build folder and other
> invalid settings. The plugin is not usable without configuring many
> details.
Yes it's not ideal that when opening a plain poky, the default 
configuration won't be compatible out of the box. However changing it 
now would break users' configs which rely on it. It can be done though, 
with a warning of some sort if necessary for old users.

We could also automatically add settings.json lines to ignore the build 
folders for CPU overload from the plugin. But I'm not sure editing 
settings.json more than we absolutely need is a good idea?

We'd also really like to have configuration snippets like you mentionned 
and stop touching settings.json. But the VSCode team has still not 
prioritized this in years.
> 
> With my patches all the issues would be addressed. The plugin just
> starts showing its real potential with the default configuration.
> 
> It also works for more complicated setups. I'm using it for example
> with these lines in a oe-init-build-env from a custom layer:
> 
> ...
> if command -v code > /dev/null; then
>      OEINITDIR=$(dirname "$THIS_SCRIPT")
>      OEINITDIR=$(readlink -f "$OEINITDIR")
>      OEINITDIR="$OEINITDIR" \
>      VSCODE_EXCLUDE_DIRS="**/a-big-folder/**" \
>      "$OEROOT"/scripts/contrib/oe-init-build-vscode.py || {
>          unset OEROOT
>          return 1
>      }
> fi
> ...
What I like with your new script is that it produces a seamless 
equivalent to the previous static config. If the static config cannot be 
versioned (I still didn't understand why), then yes, creating the file 
seems like the way to go. However, I have a few concerns with the 
current implementation:
  1. If the user has different settings, they may be corrupted by the 
script when running oe-init-build-env. My suggestion would be to not 
generate anything if a settings.json is already present.
  2. Would just having the static file somewhere and simply copying it 
work rather than assembling it? What other dynamic configurations are 
addressed with your script?

I'm not sure I understand what kind of users and workflows are targeted 
by this script and how it connects with the documentation guiding flows. 
Should we rather edit the plugin's guide example configurations for poky?
> 
> 
> Thank you and best regards,
> Adrian
> 
> 
>>>
>>> Here are some patches:
>>> https://git.yoctoproject.org/poky-contrib/log/?h=adrianf/vscode-yocto-plugin
>>>
>>> I will send them as V2 after I have tested them some more. (There
>>> are
>>> already oe-selftest)
>>>
>>> @Enguerrand your review would be appreciated.
>>>
>>> Regards,
>>> Adrian
>>>
>>>> Cheers,
>>>>
>>>> Richard
>>>
>>
>> Thanks!
>> Enguerrand de Ribaucourt
> 

Best regards,
Enguerrand de Ribaucourt
Adrian Freihofer Feb. 12, 2024, 10:43 p.m. UTC | #7
Hi Enguerrand
> 


My first solution was too complicated. After re-thinking it, I replaced
it by a much simpler implementation: The new implementation is about
extending the templates which are already in poky.

> What I like with your new script is that it produces a seamless 
> equivalent to the previous static config. If the static config cannot
> be 
> versioned (I still didn't understand why), then yes, creating the
> file 
> seems like the way to go. However, I have a few concerns with the 
> current implementation:
>   1. If the user has different settings, they may be corrupted by the
> script when running oe-init-build-env. My suggestion would be to not 
> generate anything if a settings.json is already present.

Yes, that's simple and exactly what the new implementation does.

>   2. Would just having the static file somewhere and simply copying
> it 
> work rather than assembling it? What other dynamic configurations are
> addressed with your script?
> 
Yes, that's more or less what the template does.

> I'm not sure I understand what kind of users and workflows are
> targeted 
> by this script and how it connects with the documentation guiding
> flows. 
> Should we rather edit the plugin's guide example configurations for
> poky?

I think it should just work out of the box at least for the default use
case which is also documented in the Yocto quick setup guide. And
that's what it does with the few lines of code which I just pushed to
https://git.yoctoproject.org/poky-contrib/log/?h=adrianf/vscode-yocto-plugin

What do you think?

Thank you and best regards,
Adrian
Enguerrand de Ribaucourt Feb. 13, 2024, 9:22 a.m. UTC | #8
Hello Adrian,
> Hi Enguerrand
>>
> 
> 
> My first solution was too complicated. After re-thinking it, I replaced
> it by a much simpler implementation: The new implementation is about
> extending the templates which are already in poky.
The new template mechanic is neat!
  - It provides a new way for layer maintainers to share VSCode 
configuration, including when setting up multi-layer workspaces.
  - It can be used for other build tools like kas, crops... (which I tested)
  - It works better than the previous config for plain poky. The config 
will be valid, whether the user creates the build directory inside poky 
or in a parent directory.
  - It won't corrupt existing manual settings
> 
>> What I like with your new script is that it produces a seamless
>> equivalent to the previous static config. If the static config cannot
>> be
>> versioned (I still didn't understand why), then yes, creating the
Make sure to notify us on GitHub if you come with git conflicts or 
errors, or versioning problems with the way the extension edits 
settings.json ;)
>> file
>> seems like the way to go. However, I have a few concerns with the
>> current implementation:
>>    1. If the user has different settings, they may be corrupted by the
>> script when running oe-init-build-env. My suggestion would be to not
>> generate anything if a settings.json is already present.
> 
> Yes, that's simple and exactly what the new implementation does.
> 
>>    2. Would just having the static file somewhere and simply copying
>> it
>> work rather than assembling it? What other dynamic configurations are
>> addressed with your script?
>>
> Yes, that's more or less what the template does.
It is much simpler now indeed!
> 
>> I'm not sure I understand what kind of users and workflows are
>> targeted
>> by this script and how it connects with the documentation guiding
>> flows.
>> Should we rather edit the plugin's guide example configurations for
>> poky?
> 
> I think it should just work out of the box at least for the default use
> case which is also documented in the Yocto quick setup guide. And
> that's what it does with the few lines of code which I just pushed to
> https://git.yoctoproject.org/poky-contrib/log/?h=adrianf/vscode-yocto-plugin
> 
> What do you think?
I think the current approach follows the traditional VSCode 
configuration experience:
  - How to configure the extension should be explained in the 
extension's README (we already have that)
  - There should be a mechanism to share .vscode settings for 
developers. Usually this is through having .vscode at the root of the 
git folder. But as you have explained, this is not the best solution for 
Yocto because we may have multi-layers workspaces, and also the 
settings.json edits. Your template approach satisfies both and follows 
the existing templateconf mechanic.

I'm very happy with this iteration!

Just a few minor remarks on the code:
	if command -v code > /dev/null; then
In the experience I describe just above, developers usually clone the 
.vscode settings even if they don't use them. I think we could remove 
this condition and always produce the VSCode settings. It will also 
encourage new users to try out the VSCode extension if they weren't 
aware of it, which will also be helpful to them.

You should remove the indentation in the messages "You had no 
.vscode/*.json file". cat heredocs will keep the indentation so the 
message appears indented in the output.

You also removed some the editor settings the extension edits from the 
template. I'm ok with this since the file is no longer versioned. You 
could also remove the Python analysis paths which are inserted by the 
extension. (Also it uses ${workspaceFolder} relative paths which are 
portable.)

Thanks,
> 
> Thank you and best regards,
> Adrian
> 
Enguerrand de Ribaucourt
Peter Kjellerstedt Feb. 13, 2024, 2:53 p.m. UTC | #9
> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Enguerrand de Ribaucourt
> Sent: den 13 februari 2024 10:23
> To: adrian.freihofer@gmail.com
> Cc: openembedded-core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCH] vscode: improve settings for new yocto plugin
> 
> Hello Adrian,

...

> You should remove the indentation in the messages "You had no
> .vscode/*.json file". cat heredocs will keep the indentation so the
> message appears indented in the output.

If you use `cat <<-EOF` the shell will automatically remove any tabs used 
for indentation before passing the heredoc to cat.

//Peter
diff mbox series

Patch

diff --git a/.gitignore b/.gitignore
index f6ce090b5fc..fa5345a79f5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -36,5 +36,5 @@  _toaster_clones/
 downloads/
 sstate-cache/
 toaster.sqlite
-.vscode/
-vscode-bitbake-build/
+.vscode/*
+!.vscode/settings.json
diff --git a/.vscode/settings.json b/.vscode/settings.json
index 517a86d1bfa..4d615ae3aa9 100644
--- a/.vscode/settings.json
+++ b/.vscode/settings.json
@@ -28,5 +28,31 @@ 
         "**/vscode-bitbake-build/**",
         "**/workspace/sources/**",
         "**/workspace/attic/**"
-    ]
+    ],
+    "bitbake.pathToBitbakeFolder": "${workspaceFolder}/bitbake",
+    "bitbake.pathToEnvScript": "${workspaceFolder}/oe-init-build-env",
+    "bitbake.pathToBuildFolder": "${workspaceFolder}/build",
+    "bitbake.commandWrapper": "",
+    "bitbake.workingDirectory": "",
+    "python.autoComplete.extraPaths": [
+        "${workspaceFolder}/bitbake/lib",
+        "${workspaceFolder}/meta/lib"
+    ],
+    "python.analysis.extraPaths": [
+        "${workspaceFolder}/bitbake/lib",
+        "${workspaceFolder}/meta/lib"
+    ],
+    "[python]": {
+        "diffEditor.ignoreTrimWhitespace": false,
+        "gitlens.codeLens.symbolScopes": [
+            "!Module"
+        ],
+        "editor.formatOnType": true,
+        "editor.wordBasedSuggestions": "off",
+        "files.trimTrailingWhitespace": false
+    },
+    "[shellscript]": {
+        "files.eol": "\n",
+        "files.trimTrailingWhitespace": false
+    }
 }