Skip to content
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.

Sign up for GitHub

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

Fix build cache management in workspaces #675

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Fix build cache management in workspaces #675

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ea64ab8
init
filip131311 Oct 29, 2024
d947162
init2
filip131311 Oct 29, 2024
750017b
formatting
filip131311 Oct 29, 2024
3b53a39
formatting
filip131311 Oct 29, 2024
74b439f
add universal method for storeing app speccific caches
filip131311 Oct 31, 2024
0b033a5
migrate old build caches
filip131311 Oct 31, 2024
d5412e0
formating
filip131311 Oct 31, 2024
1475e12
Update packages/vscode-extension/src/utilities/appCaches.ts
filip131311 Nov 13, 2024
bad7a58
changes after CR
filip131311 Nov 13, 2024
3e347c2
Merge branch '@Filip131311/FixFingerprintManagment' of github.com:sof…
filip131311 Nov 13, 2024
cd19698
changes after CR
filip131311 Nov 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/vscode-extension/src/builders/BuildManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ export class BuildManager {
);
}

await buildCache.storeBuild(buildFingerprint, buildResult);
await buildCache.storeCache(buildFingerprint, buildResult);
Copy link
Member

Choose a reason for hiding this comment

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

I don't like the naming being so mixed up in this whole PR. We should have naming that better maps to the intuition what cache is rather than trying to provide a super-accurate naming.

Here, it sound very weird that we are "storing cache" using "build cache". Intuitively, what this line does is it stores some build metadata (build result) in a cache storage. IMO "storeBuild" just much better describes what this method does than "storeCache".


return buildResult;
};
Expand Down
69 changes: 55 additions & 14 deletions packages/vscode-extension/src/builders/PlatformBuildCache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,17 @@
import path from "path";
import path, { join } from "path";
import fs from "fs";
import { createHash } from "crypto";
import { createFingerprintAsync } from "@expo/fingerprint";
import { Logger } from "../Logger";
import { extensionContext, getAppRootFolder } from "../utilities/extensionContext";
import { getAppRootFolder } from "../utilities/extensionContext";
import { DevicePlatform } from "../common/DeviceManager";
import { IOSBuildResult } from "./buildIOS";
import { AndroidBuildResult } from "./buildAndroid";
import { getLaunchConfiguration } from "../utilities/launchConfiguration";
import { runfingerprintCommand } from "./customBuild";
import { calculateMD5 } from "../utilities/common";
import { calculateMD5, getOrCreateBuildCachesDir } from "../utilities/common";
import { BuildResult } from "./BuildManager";

const ANDROID_BUILD_CACHE_KEY = "android_build_cache";
const IOS_BUILD_CACHE_KEY = "ios_build_cache";

const IGNORE_PATHS = [
path.join("android", ".gradle/**/*"),
path.join("android", "build/**/*"),
Expand Down Expand Up @@ -46,28 +44,60 @@ export class PlatformBuildCache {

private constructor(private readonly platform: DevicePlatform) {}

get cacheKey() {
return this.platform === DevicePlatform.Android ? ANDROID_BUILD_CACHE_KEY : IOS_BUILD_CACHE_KEY;
get appBuildCachePath() {
const appBuildCachesDir = getOrCreateAppBuildCachesDir();

let appBuildCachePath;

if (this.platform === DevicePlatform.Android) {
appBuildCachePath = join(appBuildCachesDir, "android.json");
} else {
appBuildCachePath = join(appBuildCachesDir, "ios.json");
}

return appBuildCachePath;
}

/**
* Passed fingerprint should be calculated at the time build is started.
*/
public async storeBuild(buildFingerprint: string, build: BuildResult) {
public async storeCache(buildFingerprint: string, build: BuildResult) {
Copy link
Member

Choose a reason for hiding this comment

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

why has this been renamed? The method takes fingerprint and build result that gets stored. We don't store any cache here

const appPath = await getAppHash(getAppPath(build));
await extensionContext.workspaceState.update(this.cacheKey, {

const cache = JSON.stringify({
fingerprint: buildFingerprint,
buildHash: appPath,
buildResult: build,
});

fs.writeFileSync(this.appBuildCachePath, cache);
}

public async clearCache() {
await extensionContext.workspaceState.update(this.cacheKey, undefined);
if (!fs.existsSync(this.appBuildCachePath)) {
return;
}
fs.rmSync(this.appBuildCachePath);
}

public getCache() {
if (!fs.existsSync(this.appBuildCachePath)) {
Logger.debug("No cache found.");
return undefined;
}
let cache: BuildCacheInfo;
try {
cache = JSON.parse(fs.readFileSync(this.appBuildCachePath).toString());
} catch (e) {
Logger.warn("Error parsing build cache", e);
return undefined;
}
return cache;
}

public async getBuild(currentFingerprint: string) {
const cache = extensionContext.workspaceState.get<BuildCacheInfo>(this.cacheKey);
const cache: BuildCacheInfo | undefined = this.getCache();

if (!cache) {
Logger.debug("No cached build found.");
return undefined;
Expand Down Expand Up @@ -105,8 +135,7 @@ export class PlatformBuildCache {

public async isCacheStale() {
const currentFingerprint = await this.calculateFingerprint();
const { fingerprint } =
extensionContext.workspaceState.get<BuildCacheInfo>(this.cacheKey) ?? {};
const { fingerprint } = this.getCache() ?? {};

return currentFingerprint !== fingerprint;
}
Expand Down Expand Up @@ -158,3 +187,15 @@ function getAppPath(build: BuildResult) {
async function getAppHash(appPath: string) {
return (await calculateMD5(appPath)).digest("hex");
}

function getAppRootPathHash() {
return createHash("md5").update(getAppRootFolder()).digest("hex");
}

function getOrCreateAppBuildCachesDir() {
const appBuildCachesDirLocation = join(getOrCreateBuildCachesDir(), getAppRootPathHash());
if (!fs.existsSync(appBuildCachesDirLocation)) {
fs.mkdirSync(appBuildCachesDirLocation, { recursive: true });
}
return appBuildCachesDirLocation;
}
8 changes: 8 additions & 0 deletions packages/vscode-extension/src/utilities/common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,14 @@ export function getOrCreateAppDownloadsDir() {
return downloadsDirLocation;
}

export function getOrCreateBuildCachesDir() {
Copy link
Contributor

Choose a reason for hiding this comment

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

what do you think about creating abstract utils for app root level cache management? I believe it would be enough to support storing json data? It would be nice to use something like that in #640 to store recently opened deep links

on high level it could look something like this:

// src/utilities/appCache.ts

const readAppLevelCache = (key: string): Record<string, unknown> | null => {
    return JSON.parse(fs.read(join(appCacheDir), key))); 
}

const writeAppLevelCache = (key: string, data: Record<string, unknown>) => {
    fs.write(join(appCacheDir, key), JSON.stringify(data));
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You are right it would be great to it build like that, thank you

const buildCachesDirLocation = join(getAppCachesDir(), "buildCaches");
if (!fs.existsSync(buildCachesDirLocation)) {
fs.mkdirSync(buildCachesDirLocation, { recursive: true });
}
return buildCachesDirLocation;
}

export function isDeviceIOS(deviceId: string) {
return deviceId.startsWith("ios");
}
Expand Down