feat: use Arduino CLI 1.0.4 #2457

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merged

giacomocusinato merged 5 commits into main from use-cli-1.0.0

Sep 6, 2024

Merged

feat: use Arduino CLI 1.0.4 #2457

giacomocusinato merged 5 commits into main from use-cli-1.0.0

Sep 6, 2024

Conversation

giacomocusinato

Copy link

Collaborator

@giacomocusinato giacomocusinato commented Jun 17, 2024 •

edited

Loading

Motivation

Use Arduino CLI 1.0.0 APIs in IDE2.

Change description

Update CLI version to 1.0.0, bind new APIs

Other information

Reviewer checklist

PR addresses a single concern.
The PR has no duplicates (please search among the Pull Requests before creating one)
PR title and description are properly filled.
Docs have been added / updated (for bug fixes / features)

@CLAassistant

Copy link

CLAassistant commented Jun 17, 2024 •

edited

Loading

CLA assistant check
All committers have signed the CLA.

@per1234 per1234 added type: enhancement topic: infrastructure topic: CLI labels

Jun 17, 2024

@giacomocusinato giacomocusinato force-pushed the use-cli-1.0.0 branch from 8f03dee to 540d848 Compare

June 20, 2024 14:32

@giacomocusinato giacomocusinato force-pushed the use-cli-1.0.0 branch from 540d848 to 24296c5 Compare

June 27, 2024 16:48

@giacomocusinato giacomocusinato marked this pull request as ready for review

June 27, 2024 16:48

@giacomocusinato giacomocusinato force-pushed the use-cli-1.0.0 branch from 24296c5 to 7ffd748 Compare

June 27, 2024 19:19

@giacomocusinato giacomocusinato marked this pull request as draft

July 1, 2024 18:21

@giacomocusinato


 fix: use @pingghost/protoc to compile proto files

f842bad

The npm package previously used (`protoc`) is still lacking apple arm32 support, see YePpHa/node-protoc#10

@giacomocusinato giacomocusinato force-pushed the use-cli-1.0.0 branch 2 times, most recently from d396032 to aca5ece Compare

July 12, 2024 14:42

dankeboy36

dankeboy36 reviewed

Jul 15, 2024

View reviewed changes

arduino-ide-extension/src/node/config-service-impl.ts Outdated

@@ -182,7 +182,11 @@ export class ConfigServiceImpl

});

const model = (yaml.safeLoad(content) || {}) as DefaultCliConfig;

this.logger.info(`Loaded CLI configuration: ${JSON.stringify(model)}`);

if (model.directories.data && model.directories.user) {

if (

model.directories &&

Copy link

Contributor

@dankeboy36 dankeboy36 Jul 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If directories is optional, I propose reflecting it on the type:

arduino-ide/arduino-ide-extension/src/node/cli-config.ts

Line 98 in 2e78e96

directories: Directories;

Copy link

Collaborator Author

@giacomocusinato giacomocusinato Jul 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think directories can be undefined only on the initial parse of the current config.
We can manipolate the loaded config object to assure the directories are alway defined object returned by loadCliConfig is always of type DefaultCliConfig.

That's the current logic:

Load current cli config as type CliConfig (optional directories).
Check directories value:
1. If defined, return config as DefaultCliConfig type
2. If undefined, call getFallbackCliConfig to get default directories, merge them to current config and return the object as DefaultCliConfig

arduino-ide-extension/src/node/config-service-impl.ts Outdated

Comment on lines 224 to 229

const rawJson = await spawnCommand(cliPath, ['config', 'dump', '--json']);

const config = JSON.parse(rawJson);

// Since CLI 1.0, the command `config dump` only returns user-modified values and not default ones.

// directories.user and directories.data are required by IDE2 so we get the default value explicitly.

const directoriesRaw = await spawnCommand(cliPath, [

Copy link

Contributor

@dankeboy36 dankeboy36 Jul 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CLI executions do not depend on each other: they can spawn parallel with Promise.all()

arduino-ide-extension/src/node/config-service-impl.ts Outdated

Comment on lines 321 to 316

const client = createArduinoCoreServiceClient({ port });

const req = new SettingsSetValueRequest();

Copy link

Contributor

@dankeboy36 dankeboy36 Jul 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a new CLI behavior? I see a client is created before setting each value. Can it be moved outside of the loop?

Copy link

Collaborator Author

@giacomocusinato giacomocusinato Jul 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes CLI config API's were refactored and they now only exposed methods for retrieving/setting single values, instead of the previous "merge" approach.
I agree the client should be created one time outside the loop. Thanks for the review! 👍

arduino-ide-extension/src/node/config-service-impl.ts Outdated

const cliConfigUri = await this.getCliConfigFileUri();

const cliConfigPath = FileUri.fsPath(cliConfigUri);

fs.writeFile(cliConfigPath, configRaw, { encoding: 'utf-8' });

Copy link

Contributor

@dankeboy36 dankeboy36 Jul 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing await or return

@giacomocusinato giacomocusinato marked this pull request as ready for review

July 22, 2024 08:58

@giacomocusinato giacomocusinato force-pushed the use-cli-1.0.0 branch 2 times, most recently from d78e917 to 15e2a19 Compare

July 26, 2024 12:54

@giacomocusinato giacomocusinato changed the title ~~(削除) feat: use Arduino CLI 1.0.0 (削除ここまで)~~ (追記) feat: use Arduino CLI 1.0.3 (追記ここまで)

Jul 26, 2024

giacomocusinato added 3 commits

August 20, 2024 18:24

@giacomocusinato


 feat: use Arduino CLI 1.0.4

8df2bd1

@giacomocusinato


 fix: allow use of node16 in github actions

a507bb6

@giacomocusinato


 chore: update arduino-language-server version for cli-1.0.0

92f518d

@giacomocusinato giacomocusinato force-pushed the use-cli-1.0.0 branch from 15e2a19 to 92f518d Compare

August 20, 2024 16:24

@giacomocusinato giacomocusinato changed the title ~~(削除) feat: use Arduino CLI 1.0.3 (削除ここまで)~~ (追記) feat: use Arduino CLI 1.0.4 (追記ここまで)

Aug 20, 2024

@giacomocusinato


 fix: deprecated platform order test

59b8da1

Arduino deprecated platforms should have more priority then other deprecated ones

davegarthsimpson

davegarthsimpson approved these changes

Sep 5, 2024

View reviewed changes

@giacomocusinato giacomocusinato merged commit 1ec0a8c into main

Sep 6, 2024

25 checks passed

@giacomocusinato giacomocusinato deleted the use-cli-1.0.0 branch

September 6, 2024 09:38

dankeboy36 added a commit to dankeboy36/ardunno-cli-gen that referenced this pull request

Nov 17, 2024

@dankeboy36


 chore: replace protoc with @pingghost/protoc

5fa927a

Ref: arduino/arduino-ide#2457
Signed-off-by: dankeboy36 <dankeboy36@gmail.com>

dankeboy36 added a commit to dankeboy36/ardunno-cli-gen that referenced this pull request

Nov 17, 2024

@dankeboy36


 fix: patch for missing google  proto files in CLI release asset ( #9 )

1640a42

* fix: patch for missing `google` proto files in CLI (arduino/arduino-cli#2755)
* chore: replace `protoc` with `@pingghost/protoc` (arduino/arduino-ide#2457)
* chore(ci): build on Node.js 20.x
* fix(doc): refresh docs, update protoc version
* chore: update formatter + fix example command
---------
Signed-off-by: dankeboy36 <dankeboy36@gmail.com>

Labels

topic: CLI topic: infrastructure type: enhancement

5 participants

@giacomocusinato @CLAassistant @davegarthsimpson @dankeboy36 @per1234

Uh oh!

feat: use Arduino CLI 1.0.4 #2457

feat: use Arduino CLI 1.0.4 #2457

Uh oh!

Conversation

@giacomocusinato giacomocusinato commented Jun 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Change description

Other information

Reviewer checklist

Uh oh!

CLAassistant commented Jun 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

@dankeboy36 dankeboy36 Jul 15, 2024

Choose a reason for hiding this comment

Uh oh!

@giacomocusinato giacomocusinato Jul 22, 2024

Choose a reason for hiding this comment

Uh oh!

@dankeboy36 dankeboy36 Jul 15, 2024

Choose a reason for hiding this comment

Uh oh!

@dankeboy36 dankeboy36 Jul 15, 2024

Choose a reason for hiding this comment

Uh oh!

@giacomocusinato giacomocusinato Jul 15, 2024

Choose a reason for hiding this comment

Uh oh!

@dankeboy36 dankeboy36 Jul 15, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

@giacomocusinato giacomocusinato commented Jun 17, 2024 •

edited

Loading

CLAassistant commented Jun 17, 2024 •

edited

Loading