Skip to content

Commit

Permalink
Fix #36, Downgrading SDK >= 2.0.0 with unsupported PICO_BOARD breaks …
Browse files Browse the repository at this point in the history
…cmake

Signed-off-by: paulober <[email protected]>
  • Loading branch information
paulober committed Aug 12, 2024
1 parent 08c8f07 commit e148cd4
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 31 deletions.
12 changes: 8 additions & 4 deletions src/commands/switchSDK.mts
Original file line number Diff line number Diff line change
Expand Up @@ -394,9 +394,7 @@ export default class SwitchSDKCommand extends Command {
// download and install selected SDK
if (
(await downloadAndInstallSDK(selectedSDK.sdk, SDK_REPOSITORY_URL)) &&
(await downloadAndInstallTools(
selectedSDK.sdk
))
(await downloadAndInstallTools(selectedSDK.sdk))
) {
progress.report({
increment: 40,
Expand Down Expand Up @@ -444,12 +442,18 @@ export default class SwitchSDKCommand extends Command {
increment: 10,
});

await cmakeUpdateSDK(
const cmakeUpdateResult = await cmakeUpdateSDK(
workspaceFolder.uri,
selectedSDK.sdk,
selectedToolchain.toolchain.version
);

if (!cmakeUpdateResult) {
void window.showWarningMessage(
"Failed to update CMakeLists.txt for new SDK version."
);
}

progress.report({
// show sdk installed notification
message: `Successfully installed SDK ${selectedSDK.label}.`,
Expand Down
85 changes: 58 additions & 27 deletions src/utils/cmakeUtil.mts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { readFile, writeFile } from "fs/promises";
import { rimraf, windows as rimrafWindows } from "rimraf";
import { homedir } from "os";
import which from "which";
import { compareLtMajor } from "./semverUtil.mjs";

export async function getPythonPath(): Promise<string> {
const settings = Settings.getInstance();
Expand Down Expand Up @@ -42,17 +43,15 @@ export async function getPath(): Promise<string> {

const ninjaPath = (
await which(
settings
.getString(SettingsKey.ninjaPath)
?.replace(HOME_VAR, homedir()) || "ninja",
settings.getString(SettingsKey.ninjaPath)?.replace(HOME_VAR, homedir()) ||
"ninja",
{ nothrow: true }
)
).replaceAll("\\", "/");
const cmakePath = (
await which(
settings
.getString(SettingsKey.cmakePath)
?.replace(HOME_VAR, homedir()) || "cmake",
settings.getString(SettingsKey.cmakePath)?.replace(HOME_VAR, homedir()) ||
"cmake",
{ nothrow: true }
)
).replaceAll("\\", "/");
Expand Down Expand Up @@ -80,9 +79,7 @@ export async function getPath(): Promise<string> {

const isWindows = process.platform === "win32";

return `${
ninjaPath.includes("/") ? dirname(ninjaPath) : ""
}${
return `${ninjaPath.includes("/") ? dirname(ninjaPath) : ""}${
cmakePath.includes("/")
? `${isWindows ? ";" : ":"}${dirname(cmakePath)}`
: ""
Expand All @@ -104,7 +101,7 @@ export async function configureCmakeNinja(folder: Uri): Promise<boolean> {
if (settings.getBoolean(SettingsKey.useCmakeTools)) {
await window.showErrorMessage(
"You must use the CMake Tools extension to configure your build. " +
"To use this extension instead, change the useCmakeTools setting."
"To use this extension instead, change the useCmakeTools setting."
);

return false;
Expand All @@ -131,7 +128,7 @@ export async function configureCmakeNinja(folder: Uri): Promise<boolean> {
if (p1 !== p2) {
console.warn(
`Build directory has been moved from ${p1} to ${p2}` +
` - Deleting CMakeCache.txt and regenerating.`
` - Deleting CMakeCache.txt and regenerating.`
);

rmSync(join(buildDir, "CMakeCache.txt"));
Expand Down Expand Up @@ -170,8 +167,8 @@ export async function configureCmakeNinja(folder: Uri): Promise<boolean> {
if (!customPath) {
return false;
}
customEnv[isWindows ? "Path" : "PATH"] = customPath
+ customEnv[isWindows ? "Path" : "PATH"];
customEnv[isWindows ? "Path" : "PATH"] =
customPath + customEnv[isWindows ? "Path" : "PATH"];
const pythonPath = await getPythonPath();

const command =
Expand All @@ -183,18 +180,22 @@ export async function configureCmakeNinja(folder: Uri): Promise<boolean> {
: ""
}` + `-G Ninja -B ./build "${folder.fsPath}"`;

const child = exec(command, {
env: customEnv,
cwd: folder.fsPath,
}, (error, stdout, stderr) => {
if (error) {
console.error(error);
console.log(`stdout: ${stdout}`);
console.log(`stderr: ${stderr}`);
const child = exec(
command,
{
env: customEnv,
cwd: folder.fsPath,
},
(error, stdout, stderr) => {
if (error) {
console.error(error);
console.log(`stdout: ${stdout}`);
console.log(`stderr: ${stderr}`);
}

return;
}

return;
});
);

child.on("error", err => {
console.error(err);
Expand Down Expand Up @@ -234,26 +235,28 @@ export async function cmakeUpdateSDK(
folder: Uri,
newSDKVersion: string,
newToolchainVersion: string
): Promise<void> {
): Promise<boolean> {
// TODO: support for scaning for seperate locations of the CMakeLists.txt file in the project
const cmakeFilePath = join(folder.fsPath, "CMakeLists.txt");
const sdkPathRegex = /^set\(PICO_SDK_PATH\s+([^)]+)\)$/m;
const toolchainPathRegex = /^set\(PICO_TOOLCHAIN_PATH\s+([^)]+)\)$/m;
const toolsPathRegex = /^set\(pico-sdk-tools_DIR\s+([^)]+)\)$/m;
const picoBoardRegex = /^set\(PICO_BOARD\s+([^)]+)\)$/m;

const settings = Settings.getInstance();
if (settings === undefined) {
Logger.log("Error: Settings not initialized.");

return;
return false;
}

try {
// check if CMakeLists.txt exists in the root folder
await workspace.fs.stat(folder.with({ path: cmakeFilePath }));

const content = await readFile(cmakeFilePath, "utf8");
const modifiedContent = content

var modifiedContent = content

Check failure on line 259 in src/utils/cmakeUtil.mts

View workflow job for this annotation

GitHub Actions / build

Unexpected var, use let or const instead
.replace(
sdkPathRegex,
`set(PICO_SDK_PATH \${USERHOME}/.pico-sdk/sdk/${newSDKVersion})`
Expand All @@ -268,6 +271,32 @@ export async function cmakeUpdateSDK(
`set(pico-sdk-tools_DIR \${USERHOME}/.pico-sdk/tools/${newSDKVersion})`
);

const picoBoard = content.match(picoBoardRegex);
// update the PICO_BOARD variable if it's a pico2 board and the new sdk version is less than 2.0.0

Check warning on line 275 in src/utils/cmakeUtil.mts

View workflow job for this annotation

GitHub Actions / build

This line has a comment length of 102. Maximum allowed is 100
if (
picoBoard !== null &&
picoBoard[1].includes("pico2") &&
compareLtMajor(newSDKVersion, "2.0.0")
) {
const result = await window.showQuickPick(["pico", "pico_w"], {
placeHolder: "The new SDK version does not support your current board",
canPickMany: false,
ignoreFocusOut: true,
title: "Please select a new board type",
});

if (result === undefined) {
Logger.log("User canceled board type selection during SDK update.");

return false;
}

modifiedContent = modifiedContent.replace(
picoBoardRegex,
`set(PICO_BOARD ${result} CACHE STRING "Board type")`
);
}

await writeFile(cmakeFilePath, modifiedContent, "utf8");
Logger.log("Updated paths in CMakeLists.txt successfully.");

Expand All @@ -283,8 +312,10 @@ export async function cmakeUpdateSDK(
await configureCmakeNinja(folder);

Logger.log("Reconfigured CMake successfully.");
return true;

Check warning on line 315 in src/utils/cmakeUtil.mts

View workflow job for this annotation

GitHub Actions / build

Expected newline before return statement
} catch (error) {
Logger.log("Error updating paths in CMakeLists.txt!");
return false;

Check warning on line 318 in src/utils/cmakeUtil.mts

View workflow job for this annotation

GitHub Actions / build

Expected newline before return statement
}
}

Expand Down
67 changes: 67 additions & 0 deletions src/utils/semverUtil.mts
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,70 @@ export function compareGe(first: string, second: string): boolean {
// If all parts are equal
return true;
}

/**
* Function to compare two semantic version strings.
* Returns true if the first version is less than the second version.
*
* @param first - The first version string.
* @param second - The second version string.
* @returns boolean - True if the first version is less than the second version.
*/
export function compareLt(first: string, second: string): boolean {
// Split the version strings into parts and convert them to numbers
const firstParts = first.split(".").map(Number);
const secondParts = second.split(".").map(Number);

// Ensure both versions have three parts
if (firstParts.length !== 3 || secondParts.length !== 3) {
throw new Error(
"Version strings must have exactly three parts (major.minor.patch)"
);
}

// Compare the version parts directly
for (let i = 0; i < 3; i++) {
if (firstParts[i] < secondParts[i]) {
return true;
} else if (firstParts[i] > secondParts[i]) {
return false;
}
}

// If all parts are equal, first is not less than second
return false;
}

/**
* Function to compare the major versions of two semantic version strings.
* Returns true if the first version's major part is greater than or equal to the second version's major part.

Check warning on line 110 in src/utils/semverUtil.mts

View workflow job for this annotation

GitHub Actions / build

This line has a comment length of 110. Maximum allowed is 100
*
* @param first - The first version string.
* @param second - The second version string.
* @returns boolean - True if the first version's major part is greater than or equal to the second version's major part.

Check warning on line 114 in src/utils/semverUtil.mts

View workflow job for this annotation

GitHub Actions / build

This line has a comment length of 121. Maximum allowed is 100
*/
export function compareGeMajor(first: string, second: string): boolean {
// Split the version strings into parts and convert them to numbers
const firstMajor = parseInt(first.split(".")[0]);
const secondMajor = parseInt(second.split(".")[0]);

// Compare only the major versions
return firstMajor >= secondMajor;
}

/**
* Function to compare the major versions of two semantic version strings.
* Returns true if the first version's major part is less than the second version's major part.
*
* @param first - The first version string.
* @param second - The second version string.
* @returns boolean - True if the first version's major part is less than the second version's major part.

Check warning on line 131 in src/utils/semverUtil.mts

View workflow job for this annotation

GitHub Actions / build

This line has a comment length of 106. Maximum allowed is 100
*/
export function compareLtMajor(first: string, second: string): boolean {
// Split the version strings into parts and convert them to numbers
const firstMajor = parseInt(first.split(".")[0]);
const secondMajor = parseInt(second.split(".")[0]);

// Compare only the major versions
return firstMajor < secondMajor;
}

0 comments on commit e148cd4

Please sign in to comment.