Skip to content

Commit fcbc421

Browse files
jthuangarmjthuangarm
authored
Merge branch 'main' into clean-pr
2 parents fa1a894 + fa41080 commit fcbc421

2 files changed

Lines changed: 174 additions & 82 deletions

File tree

‎src/solutions/solution-converter.test.ts‎

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,13 +220,31 @@ describe('SolutionConverter', () => {
220220
expect(completedListener).toHaveBeenCalledTimes(1);
221221
});
222222

223-
it('prints an error message to the output channel if the solution could not be converted', async () => {
223+
it('prints an error message when ListMissingPacks fails and ConvertSolution is skipped', async () => {
224224
mockCsolutionService.listMissingPacks.mockResolvedValue({ success: false });
225225
mockCsolutionService.convertSolution.mockResolvedValue({ success: false });
226226
await fireAndWaitForConversion();
227227

228228
const outputChannel = outputChannelProvider.mockGetCreatedChannelByName(manifest.CMSIS_SOLUTION_OUTPUT_CHANNEL);
229229

230+
// When listMissingPacks fails, ConvertSolution is skipped
231+
expect(outputChannel!.mockAppendedStrings).toEqual([
232+
expect.stringContaining('⚙️ Converting solution...'),
233+
expect.stringContaining('Check for missing packs...'),
234+
expect.stringContaining('Get log messages...'),
235+
expect.stringContaining('🟥 Convert solution failed'),
236+
]);
237+
238+
expect(completedListener).toHaveBeenCalledTimes(1);
239+
});
240+
241+
it('prints an error when convert solution fails', async () => {
242+
mockCsolutionService.listMissingPacks.mockResolvedValue({ success: true });
243+
mockCsolutionService.convertSolution.mockResolvedValue({ success: false });
244+
await fireAndWaitForConversion();
245+
246+
const outputChannel = outputChannelProvider.mockGetCreatedChannelByName(manifest.CMSIS_SOLUTION_OUTPUT_CHANNEL);
247+
230248
expect(outputChannel!.mockAppendedStrings).toEqual([
231249
expect.stringContaining('⚙️ Converting solution...'),
232250
expect.stringContaining('Check for missing packs...'),
@@ -303,7 +321,7 @@ describe('SolutionConverter', () => {
303321
expect(completedListener).toHaveBeenCalledTimes(1);
304322
});
305323

306-
it('get cbuild output and set diagnostics accordingly', async () => {
324+
it('get cbuild west output and set diagnostics accordingly', async () => {
307325
const mockDiagnosticsCollectionSet = jest.spyOn(vscode.languages.createDiagnosticCollection(), 'set');
308326
mockCsolutionService.convertSolution.mockResolvedValue({ success: true });
309327
mockCsolutionService.getLogMessages.mockResolvedValue({ success: true });
@@ -406,4 +424,40 @@ describe('SolutionConverter', () => {
406424
);
407425
});
408426
});
427+
428+
it('creates diagnostic when cpackget fails to download a pack', async () => {
429+
const diagnosticCollection = vscode.languages.createDiagnosticCollection();
430+
const mockDiagnosticsCollectionSet = jest.spyOn(diagnosticCollection, 'set') as unknown as jest.MockedFunction<
431+
(uri: vscode.Uri, diagnostics: readonly vscode.Diagnostic[] | undefined) => void
432+
>;
433+
jest.spyOn(cmsisToolboxManager, 'runCmsisTool').mockImplementation(async (_t, _a, onOutput) => {
434+
onOutput('W: retry failed');
435+
onOutput('E: network timeout');
436+
return [1, undefined];
437+
});
438+
mockCsolutionService.listMissingPacks.mockResolvedValue({ success: true, packs: ['VendorA::PackA@1.0.0'] });
439+
mockCsolutionService.getLogMessages.mockResolvedValue({ success: true });
440+
441+
await fireAndWaitForConversion();
442+
443+
expect(mockDiagnosticsCollectionSet).toHaveBeenCalledTimes(1);
444+
const [[, diagnostics]] = mockDiagnosticsCollectionSet.mock.calls;
445+
expect(diagnostics?.[0]?.message).toContain('network timeout');
446+
expect(diagnostics?.[0]?.message).toContain('retry failed');
447+
});
448+
449+
it('extracts warnings from cbuild2cmake and csolution tool output', async () => {
450+
const mockDiagnosticsCollectionSet = jest.spyOn(vscode.languages.createDiagnosticCollection(), 'set');
451+
mockCsolutionService.convertSolution.mockResolvedValue({ success: true });
452+
mockCsolutionService.getLogMessages.mockResolvedValue({ success: true });
453+
jest.spyOn(compileCommandsGenerator, 'runCbuildSetup').mockResolvedValue([true, [
454+
'warning cbuild2cmake: some warning',
455+
'error csolution: some error',
456+
]]);
457+
458+
await fireAndWaitForConversion();
459+
460+
// Expect two calls: one for cbuild2cmake warning, one for csolution error
461+
expect(mockDiagnosticsCollectionSet).toHaveBeenCalledTimes(2);
462+
});
409463
});

‎src/solutions/solution-converter.ts‎

Lines changed: 118 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -123,52 +123,63 @@ export class SolutionConverterImpl implements SolutionConverter {
123123
return;
124124
}
125125

126+
let toolsOutputMessages: string[] = [];
127+
126128
outputChannel.appendLine('⚙️ Converting solution...');
129+
let missingPacksResult = undefined;
127130
if (this.isDownloadPacksEnabled()) {
128131
// rpc method: ListMissingPacks
129132
outputChannel.append('Check for missing packs... ');
130-
const result = await this.cmsisToolboxManager.runCsolutionRpc(
133+
missingPacksResult = await this.cmsisToolboxManager.runCsolutionRpc(
131134
'ListMissingPacks',
132135
{
133136
solution: activeSolution,
134137
activeTarget: activeTarget,
135138
}
136139
) as rpc.ListMissingPacksResult;
137-
if (result.success && result.packs && result.packs.length > 0) {
140+
if (missingPacksResult.success && missingPacksResult.packs && missingPacksResult.packs.length > 0) {
138141
// download missing packs if any
139-
await this.downloadMissingPacks(result.packs);
142+
const downloadPacksOutput = await this.downloadMissingPacks(missingPacksResult.packs);
143+
toolsOutputMessages = toolsOutputMessages.concat(downloadPacksOutput);
144+
missingPacksResult.success = downloadPacksOutput.length === 0;
140145
}
141146
}
142147
if (signal.aborted) {
143148
return;
144149
}
145150

146-
// rpc method: ConvertSolution
147-
outputChannel.append('Convert solution... ');
148-
const result = await this.cmsisToolboxManager.runCsolutionRpc(
149-
'ConvertSolution',
150-
{
151-
solution: activeSolution,
152-
activeTarget: activeTarget,
153-
updateRte: this.data.updateRte ?? false,
151+
let detection = false;
152+
let convertResult: rpc.ConvertSolutionResult = { success: false };
153+
const csolution = this.solutionManager.getCsolution();
154+
if (!missingPacksResult || missingPacksResult.success) {
155+
// rpc method: ConvertSolution
156+
outputChannel.append('Convert solution... ');
157+
convertResult = await this.cmsisToolboxManager.runCsolutionRpc(
158+
'ConvertSolution',
159+
{
160+
solution: activeSolution,
161+
activeTarget: activeTarget,
162+
updateRte: this.data.updateRte ?? false,
163+
}
164+
) as rpc.ConvertSolutionResult;
165+
166+
if (signal.aborted) {
167+
return;
154168
}
155-
) as rpc.ConvertSolutionResult;
156169

157-
if (signal.aborted) {
158-
return;
170+
// compilers and variables detection handling: apply select-compiler and discover layer configurations if any
171+
csolution?.setSelectCompiler(convertResult.selectCompiler);
172+
detection = (!!convertResult.undefinedLayers && await this.checkDiscoverLayers()) || !!convertResult.selectCompiler;
159173
}
160-
// compilers and variables detection handling: apply select-compiler and discover layer configurations if any
161-
const csolution = this.solutionManager.getCsolution();
162-
csolution?.setSelectCompiler(result.selectCompiler);
163-
const detection = (!!result.undefinedLayers && await this.checkDiscoverLayers()) || !!result.selectCompiler;
164174

165-
let cbuildOutput = undefined;
166175
let logResult = undefined;
167176
if (!detection) {
168-
if (result.success) {
177+
if (convertResult.success) {
169178
// check if compile commands need to be updated: call cbuild setup skipping csolution convert step
170179
outputChannel.append('Setup database... ');
171-
[result.success, cbuildOutput] = await this.compileCommandsGenerator.runCbuildSetup();
180+
let cbuildOutput = undefined;
181+
[convertResult.success, cbuildOutput] = await this.compileCommandsGenerator.runCbuildSetup();
182+
toolsOutputMessages = toolsOutputMessages.concat(cbuildOutput ?? []);
172183
}
173184
// rpc method: GetLogMessages
174185
outputChannel.append('Get log messages... ');
@@ -184,8 +195,8 @@ export class SolutionConverterImpl implements SolutionConverter {
184195
return;
185196
}
186197
// update 'problems' view
187-
logResult = { errors: [], warnings: [], info: [], ...logResult, success: result.success };
188-
const severity = await this.updateDiagnostics(logResult, cbuildOutput);
198+
logResult = { errors: [], warnings: [], info: [], ...logResult, success: convertResult.success };
199+
const severity = await this.updateDiagnostics(logResult, toolsOutputMessages);
189200

190201
csolution?.setLogMessages(logResult);
191202

@@ -213,14 +224,29 @@ export class SolutionConverterImpl implements SolutionConverter {
213224
}
214225
}
215226

216-
private async downloadMissingPacks(packs: string[]): Promise<void> {
227+
private async downloadMissingPacks(packs: string[]): Promise<string[]> {
217228
// call cpackget to download missing packs
218229
const outputChannel = this.outputChannelProvider.getOrCreate(manifest.CMSIS_SOLUTION_OUTPUT_CHANNEL);
219230
outputChannel.append('Downloading missing packs...\n');
231+
const formattedOutput: string[] = [];
220232
for (const pack of packs) {
233+
const cpackgetOutput: string[] = [];
221234
const args = ['add', pack, '--force-reinstall', '--agree-embedded-license', '--no-dependencies'];
222-
await this.cmsisToolboxManager.runCmsisTool('cpackget', args, line => outputChannel.appendLine(line.trimEnd()), undefined, undefined, true);
235+
const [returnCode] = await this.cmsisToolboxManager.runCmsisTool('cpackget', args, line => {
236+
line = line.trimEnd();
237+
cpackgetOutput.push(line);
238+
outputChannel.appendLine(line);
239+
}, undefined, undefined, true);
240+
if (returnCode !== 0) {
241+
let errorMessage = `error cpackget: downloading pack '${pack}' failed`;
242+
const details = cpackgetOutput.join('\n').match(/[EW]: .*/g)?.map(line => line.replace(/^[EW]:\s*/, '')).join('\n') ?? '';
243+
if (details) {
244+
errorMessage += `\n${details}`;
245+
}
246+
formattedOutput.push(errorMessage);
247+
}
223248
}
249+
return formattedOutput;
224250
}
225251

226252
private async checkDiscoverLayers(): Promise<boolean> {
@@ -291,14 +317,14 @@ export class SolutionConverterImpl implements SolutionConverter {
291317
return false;
292318
}
293319

294-
private async updateDiagnostics(messages: rpc.LogMessages, cbuildOutput?: string[]): Promise<Severity> {
320+
private async updateDiagnostics(messages: rpc.LogMessages, toolsOutputMessages?: string[]): Promise<Severity> {
295321
// clear previous diagnostics
296322
this.diagnosticCollection.clear();
297323
this.collectYmlFiles();
298324
let diagnostics = false;
299325

300-
// extract cbuild messages from cbuild output
301-
await this.collectCbuildMessages(messages, cbuildOutput);
326+
// extract messages from tools output
327+
await this.collectToolsMessages(messages, toolsOutputMessages);
302328

303329
// iterate through log messages and set diagnostics
304330
for (const message of messages.errors ?? []) {
@@ -419,26 +445,11 @@ export class SolutionConverterImpl implements SolutionConverter {
419445
}
420446

421447
/**
422-
* patterns to extract env vars warnings from cbuild output
448+
* patterns to extract errors and warnings from tools messages
423449
*/
424-
private readonly cbuildWarningPatterns = [
425-
/^warning cbuild: missing ([A-Za-z_][A-Za-z0-9_]*) environment variable$/,
426-
/^warning cbuild: ([A-Za-z_][A-Za-z0-9_]*) environment variable specifies non-existent directory: .+$/
427-
];
428-
429-
/**
430-
* patterns to extract env vars errors from cbuild output
431-
*/
432-
private readonly cbuildErrorPatterns = [
433-
/^error cbuild: exec: "west": executable file not found in .+$/,
434-
];
435-
436-
/**
437-
* patterns to remove from extracted cbuild messages
438-
*/
439-
private readonly cbuildPrefixPatterns = {
440-
error: /^error cbuild:\s*/,
441-
warning: /^warning cbuild:\s*/,
450+
private readonly toolsPrefixPatterns = {
451+
error: /^.*error (?:cbuild|cbuild2cmake|csolution|cpackget):\s*/,
452+
warning: /^.*warning (?:cbuild|cbuild2cmake|csolution|cpackget):\s*/,
442453
};
443454

444455
private pushUniquely(array: string[], value: string) {
@@ -447,45 +458,72 @@ export class SolutionConverterImpl implements SolutionConverter {
447458
}
448459
}
449460

450-
private async collectCbuildMessages(logMessages: rpc.LogMessages, lines?: string[]): Promise<void> {
451-
if (!lines) {
452-
return;
461+
private async collectToolsMessages(logMessages: rpc.LogMessages, lines?: string[]): Promise<void> {
462+
if (lines) {
463+
let errors = lines.filter(line =>
464+
this.toolsPrefixPatterns.error.test(line)
465+
);
466+
let warnings = lines.filter(line =>
467+
this.toolsPrefixPatterns.warning.test(line)
468+
);
469+
if (warnings.length || errors.length) {
470+
// remove tool-specific prefixes from messages
471+
const sanitize = (m: string, kind: 'error' | 'warning') => m.replace(this.toolsPrefixPatterns[kind], '').trim();
472+
errors = errors.map(e => sanitize(e, 'error'));
473+
warnings = warnings.map(w => sanitize(w, 'warning'));
474+
// format west related messages if any
475+
await this.formatWestMessages(errors, warnings);
476+
// append messages to logMessages
477+
errors.forEach(e => this.pushUniquely(logMessages.errors ?? [], e));
478+
warnings.forEach(w => this.pushUniquely(logMessages.warnings ?? [], w));
479+
}
453480
}
454-
// extract cbuild warnings and errors around environment variables settings
455-
let errors = lines.filter(line =>
456-
this.cbuildErrorPatterns.some(pattern => pattern.test(line))
457-
);
458-
let warnings = lines.filter(line =>
459-
this.cbuildWarningPatterns.some(pattern => pattern.test(line))
481+
}
482+
483+
/**
484+
* patterns to extract environment variables and west warnings and errors
485+
*/
486+
private readonly envVarWestPatterns = [
487+
/^missing ([A-Za-z_][A-Za-z0-9_]*) environment variable$/,
488+
/^([A-Za-z_][A-Za-z0-9_]*) environment variable specifies non-existent directory: .+$/,
489+
/^exec: "west": executable file not found in .+$/,
490+
];
491+
492+
private async formatWestMessages(errors: string[], warnings: string[]): Promise<void> {
493+
// extract warnings and errors around environment variables and west settings
494+
const hasWestMessages = [...errors, ...warnings].some(line =>
495+
this.envVarWestPatterns.some(pattern => pattern.test(line))
460496
);
461-
if (!warnings.length && !errors.length) {
497+
if (!hasWestMessages) {
462498
return;
463499
}
464-
// find cmsis-csolution.environmentVariables location in settings.json
465500
const workspaceFolder = getWorkspaceFolder();
466-
if (workspaceFolder) {
467-
const settings = path.join(workspaceFolder, '.vscode', 'settings.json');
468-
const envvars = '"cmsis-csolution.environmentVariables"';
469-
let startPos: vscode.Position | undefined = undefined;
470-
if (fsUtils.fileExists(settings)) {
471-
const doc = await vscode.workspace.openTextDocument(settings);
472-
if (doc) {
473-
const startOffset = doc.getText().indexOf(envvars);
474-
if (startOffset > 0) {
475-
startPos = doc.positionAt(startOffset);
476-
}
477-
}
501+
if (!workspaceFolder) {
502+
return;
503+
}
504+
// find cmsis-csolution.environmentVariables location in workspace file or settings.json
505+
const settings = vscode.workspace.workspaceFile?.fsPath ?? path.join(workspaceFolder, '.vscode', 'settings.json');
506+
const settingsName = getFileNameFromPath(settings);
507+
const envvars = '"cmsis-csolution.environmentVariables"';
508+
let startPos: vscode.Position | undefined;
509+
if (fsUtils.fileExists(settings)) {
510+
const doc = await vscode.workspace.openTextDocument(settings);
511+
const startOffset = doc.getText().indexOf(envvars);
512+
if (startOffset >= 0) {
513+
startPos = doc.positionAt(startOffset);
478514
}
479-
const location = startPos ? `:${startPos.line + 1}:${startPos.character + 1}` : '';
480-
// format messages
481-
const format = (m: string, kind: 'error' | 'warning') =>
482-
`settings.json${location} - ${m.replace(this.cbuildPrefixPatterns[kind], '')}; review ${envvars}`;
483-
errors = errors.map(m => format(m, 'error'));
484-
warnings = warnings.map(m => format(m, 'warning'));
485-
this.addFile(settings);
486515
}
487-
// append formatted messages to logMessages
488-
errors.forEach(e => this.pushUniquely(logMessages.errors ?? [], e));
489-
warnings.forEach(w => this.pushUniquely(logMessages.warnings ?? [], w));
516+
const location = startPos ? `:${startPos.line + 1}:${startPos.character + 1}` : '';
517+
// format messages
518+
const format = (items: string[]) => {
519+
for (let i = 0; i < items.length; i++) {
520+
if (this.envVarWestPatterns.some(pattern => pattern.test(items[i]))) {
521+
items[i] = `${settingsName}${location} - ${items[i]}; review ${envvars}`;
522+
}
523+
}
524+
};
525+
format(errors);
526+
format(warnings);
527+
this.addFile(settings);
490528
}
491529
}

0 commit comments

Comments
 (0)