Fixes for importing project bundles (#7558)

This fixes a few issues:
* support for `enso-project` bundles that were compressed in a way that includes leading `./` in the paths;
* partially undos #7305 — projects won't be renamed on import.

Many thanks to @somebody1234 for the help.
This commit is contained in:
Michał Wawrzyniec Urbańczyk 2023-08-11 18:30:49 +02:00 committed by GitHub
parent 9f4a5f90c0
commit 87f5ea021f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 103 additions and 143 deletions

View File

@ -12,8 +12,6 @@ import createServer from 'create-servers'
import * as common from 'enso-common'
import * as contentConfig from 'enso-content-config'
import * as projectManagement from 'project-management'
import * as paths from '../paths'
import GLOBAL_CONFIG from '../../../../../gui/config.yaml' assert { type: 'yaml' }
@ -33,7 +31,7 @@ const HTTP_STATUS_NOT_FOUND = 404
/** External functions for a {@link Server}. */
export interface ExternalFunctions {
uploadProjectBundle: (project: stream.Readable) => Promise<projectManagement.BundleInfo>
uploadProjectBundle: (project: stream.Readable) => Promise<string>
}
/** Constructor parameter for the server configuration. */
@ -154,15 +152,14 @@ export class Server {
// When accessing the app from Electron, the file input event will have the
// full system path.
case '/api/upload-project': {
void this.config.externalFunctions.uploadProjectBundle(request).then(info => {
const body = JSON.stringify(info)
void this.config.externalFunctions.uploadProjectBundle(request).then(id => {
response
.writeHead(HTTP_STATUS_OK, [
['Content-Length', `${body.length}`],
['Content-Type', 'application/json'],
['Content-Length', `${id.length}`],
['Content-Type', 'text/plain'],
...common.COOP_COEP_CORP_HEADERS,
])
.end(body)
.end(id)
})
break
}

View File

@ -158,7 +158,7 @@ export function setOpenFileEventHandler(setProjectToOpen: (id: string) => void)
* @throws {Error} if the project from the file cannot be opened or imported. */
export function handleOpenFile(openedFile: string): string {
try {
return project.importProjectFromPath(openedFile).id
return project.importProjectFromPath(openedFile)
} catch (error) {
// Since the user has explicitly asked us to open a file, in case of an error, we should
// display a message box with the error details.

View File

@ -41,5 +41,3 @@ export const PROJECT_MANAGER_PATH = path.join(
/** Relative path of Enso Project PM metadata relative to project's root. */
export const PROJECT_METADATA_RELATIVE = path.join('.enso', 'project.json')
/** Relative path of Enso Project bundle metadata relative to project's root. */
export const BUNDLE_METADATA_RELATIVE = path.join('package.yaml')

View File

@ -6,7 +6,6 @@
import * as electron from 'electron'
import * as ipc from 'ipc'
import * as projectManagement from 'project-management'
// =================
// === Constants ===
@ -23,15 +22,12 @@ const AUTHENTICATION_API_KEY = 'authenticationApi'
// === importProjectFromPath ===
// =============================
const IMPORT_PROJECT_RESOLVE_FUNCTIONS = new Map<
string,
(projectId: projectManagement.BundleInfo) => void
>()
const IMPORT_PROJECT_RESOLVE_FUNCTIONS = new Map<string, (projectId: string) => void>()
electron.contextBridge.exposeInMainWorld(BACKEND_API_KEY, {
importProjectFromPath: (projectPath: string) => {
electron.ipcRenderer.send(ipc.Channel.importProjectFromPath, projectPath)
return new Promise<projectManagement.BundleInfo>(resolve => {
return new Promise<string>(resolve => {
IMPORT_PROJECT_RESOLVE_FUNCTIONS.set(projectPath, resolve)
})
},
@ -39,10 +35,10 @@ electron.contextBridge.exposeInMainWorld(BACKEND_API_KEY, {
electron.ipcRenderer.on(
ipc.Channel.importProjectFromPath,
(_event, projectPath: string, projectInfo: projectManagement.BundleInfo) => {
(_event, projectPath: string, projectId: string) => {
const resolveFunction = IMPORT_PROJECT_RESOLVE_FUNCTIONS.get(projectPath)
IMPORT_PROJECT_RESOLVE_FUNCTIONS.delete(projectPath)
resolveFunction?.(projectInfo)
resolveFunction?.(projectId)
}
)

View File

@ -28,19 +28,13 @@ const logger = config.logger
// === Project Import ===
// ======================
/** Information required to display a project bundle. */
export interface BundleInfo {
name: string
id: string
}
/** Open a project from the given path. Path can be either a source file under the project root,
* or the project bundle. If needed, the project will be imported into the Project Manager-enabled
* location.
*
* @returns Project ID (from Project Manager's metadata) identifying the imported project.
* @throws {Error} if the path does not belong to a valid project. */
export function importProjectFromPath(openedPath: string): BundleInfo {
export function importProjectFromPath(openedPath: string): string {
if (pathModule.extname(openedPath).endsWith(fileAssociations.BUNDLED_PROJECT_SUFFIX)) {
logger.log(`Path '${openedPath}' denotes a bundled project.`)
// The second part of condition is for the case when someone names a directory
@ -70,13 +64,25 @@ export function importProjectFromPath(openedPath: string): BundleInfo {
/** Import the project from a bundle.
*
* @returns Project ID (from Project Manager's metadata) identifying the imported project. */
export function importBundle(bundlePath: string): BundleInfo {
export function importBundle(bundlePath: string): string {
logger.log(`Importing project '${bundlePath}' from bundle.`)
// The bundle is a tarball, so we just need to extract it to the right location.
const bundleRoot = directoryWithinBundle(bundlePath)
const target = generateDirectoryName(bundleRoot ?? bundlePath)
logger.log(`Importing project as '${target.name}'.`)
fs.mkdirSync(target.path, { recursive: true })
const bundlePrefix = prefixInBundle(bundlePath)
// We care about spurious '.' and '..' when stripping paths but not when generating name.
const normalizedBundlePrefix =
bundlePrefix != null
? pathModule.normalize(bundlePrefix).replace(/[\\/]+$/, '') // Also strip trailing slash.
: null
const dirNameBase =
normalizedBundlePrefix != null &&
normalizedBundlePrefix !== '.' &&
normalizedBundlePrefix !== '..'
? normalizedBundlePrefix
: bundlePath
logger.log(`Bundle normalized prefix: '${String(normalizedBundlePrefix)}'.`)
const targetPath = generateDirectoryName(dirNameBase)
logger.log(`Importing project as '${targetPath}'.`)
fs.mkdirSync(targetPath, { recursive: true })
// To be more resilient against different ways that user might attempt to create a bundle,
// we try to support both archives that:
// * contain a single directory with the project files - that directory name will be used
@ -86,47 +92,51 @@ export function importBundle(bundlePath: string): BundleInfo {
// We try to tell apart these two cases by looking at the common prefix of the paths
// of the files in the archive. If there is any, everything is under a single directory,
// and we need to strip it.
//
// Additionally, we need to take into account that paths might be prefixed with `./` or not.
// Thus, we need to adjust the number of path components to strip accordingly.
logger.log(`Extracting bundle: '${bundlePath}' -> '${targetPath}'.`)
// Strip trailing separator and split the path into pieces.
const rootPieces = bundlePrefix != null ? bundlePrefix.split(/[\\/]/) : []
// If the last element is empty string (i.e. we had trailing separator), drop it.
if (rootPieces.length > 0 && rootPieces[rootPieces.length - 1] === '') {
rootPieces.pop()
}
tar.extract({
file: bundlePath,
cwd: target.path,
cwd: targetPath,
sync: true,
strip: bundleRoot != null ? 1 : 0,
strip: rootPieces.length,
})
updateName(target.path, target.name)
return { name: target.name, id: updateIdAndDate(target.path) }
return updateIdAndDate(targetPath)
}
/** Upload the project from a bundle. */
export async function uploadBundle(bundle: stream.Readable): Promise<BundleInfo> {
export async function uploadBundle(bundle: stream.Readable): Promise<string> {
logger.log(`Uploading project from bundle.`)
let target = generateDirectoryName('Project')
fs.mkdirSync(target.path, { recursive: true })
const targetPath = generateDirectoryName('Project')
fs.mkdirSync(targetPath, { recursive: true })
await new Promise<void>(resolve => {
bundle.pipe(tar.extract({ cwd: target.path })).on('finish', resolve)
bundle.pipe(tar.extract({ cwd: targetPath })).on('finish', resolve)
})
const entries = fs.readdirSync(target.path)
const entries = fs.readdirSync(targetPath)
const firstEntry = entries[0]
// If the directory only contains one subdirectory, replace the directory with its sole
// subdirectory.
if (entries.length === 1 && firstEntry != null) {
if (fs.statSync(pathModule.join(target.path, firstEntry)).isDirectory()) {
if (fs.statSync(pathModule.join(targetPath, firstEntry)).isDirectory()) {
const temporaryDirectoryName =
target.path + `_${crypto.randomUUID().split('-')[0] ?? ''}`
fs.renameSync(target.path, temporaryDirectoryName)
fs.renameSync(pathModule.join(temporaryDirectoryName, firstEntry), target.path)
targetPath + `_${crypto.randomUUID().split('-')[0] ?? ''}`
fs.renameSync(targetPath, temporaryDirectoryName)
fs.renameSync(pathModule.join(temporaryDirectoryName, firstEntry), targetPath)
fs.rmdirSync(temporaryDirectoryName)
}
}
const projectName = tryGetName(target.path)
if (projectName != null) {
const oldPath = target.path
target = generateDirectoryName(projectName)
if (target.path !== oldPath) {
fs.renameSync(oldPath, target.path)
}
}
updateName(target.path, target.name)
return { name: target.name, id: updateIdAndDate(target.path) }
return updateIdAndDate(targetPath)
}
/** Import the project so it becomes visible to the Project Manager.
@ -134,28 +144,27 @@ export async function uploadBundle(bundle: stream.Readable): Promise<BundleInfo>
* @param rootPath - The path to the project root.
* @returns The project ID (from the Project Manager's metadata) identifying the imported project.
* @throws {Error} if a race condition occurs when generating a unique project directory name. */
export function importDirectory(rootPath: string): BundleInfo {
export function importDirectory(rootPath: string): string {
if (isProjectInstalled(rootPath)) {
// Project is already visible to Project Manager, so we can just return its ID.
logger.log(`Project already installed at '${rootPath}'.`)
const id = getProjectId(rootPath)
if (id != null) {
return { name: getProjectName(rootPath), id }
return id
} else {
throw new Error(`Project already installed, but missing metadata.`)
}
} else {
logger.log(`Importing a project copy from '${rootPath}'.`)
const target = generateDirectoryName(rootPath)
if (fs.existsSync(target.path)) {
throw new Error(`Project directory '${target.path}' already exists.`)
const targetPath = generateDirectoryName(rootPath)
if (fs.existsSync(targetPath)) {
throw new Error(`Project directory '${targetPath}' already exists.`)
} else {
logger.log(`Copying: '${rootPath}' -> '${target.path}'.`)
fs.cpSync(rootPath, target.path, { recursive: true })
updateName(target.path, target.name)
logger.log(`Copying: '${rootPath}' -> '${targetPath}'.`)
fs.cpSync(rootPath, targetPath, { recursive: true })
// Update the project ID, so we are certain that it is unique.
// This would be violated, if we imported the same project multiple times.
return { name: target.name, id: updateIdAndDate(target.path) }
return updateIdAndDate(targetPath)
}
}
}
@ -247,45 +256,32 @@ export function updateMetadata(
/** Check if the given path represents the root of an Enso project.
* This is decided by the presence of the Project Manager's metadata. */
export function isProjectRoot(candidatePath: string): boolean {
const packageYamlPath = pathModule.join(candidatePath, paths.BUNDLE_METADATA_RELATIVE)
const projectJsonPath = pathModule.join(candidatePath, paths.PROJECT_METADATA_RELATIVE)
let isRoot = false
try {
fs.accessSync(packageYamlPath, fs.constants.R_OK)
fs.accessSync(projectJsonPath, fs.constants.R_OK)
return true
} catch {
try {
fs.accessSync(projectJsonPath, fs.constants.R_OK)
isRoot = true
} catch {
// No need to do anything, isRoot is already set to false
}
return false
}
return isRoot
}
/** Check if this bundle is a compressed directory (rather than directly containing the project
* files). If it is, we return the name of the directory. Otherwise, we return `null`. */
export function directoryWithinBundle(bundlePath: string): string | null {
* files). If it is, we return the path to the directory. Otherwise, we return `null`. */
export function prefixInBundle(bundlePath: string): string | null {
// We need to look up the root directory among the tarball entries.
let commonPrefix: string | null = null
tar.list({
file: bundlePath,
sync: true,
onentry: entry => {
// We normalize to get rid of leading `.` (if any).
const path = entry.path.normalize()
const path = entry.path
commonPrefix = commonPrefix == null ? path : utils.getCommonPrefix(commonPrefix, path)
},
})
// ESLint doesn't know that `commonPrefix` can be not `null` here due to the `onentry` callback.
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
return commonPrefix != null && commonPrefix !== '' ? pathModule.basename(commonPrefix) : null
}
/** An object containing the name and path of a project. */
interface NameAndPath {
name: string
path: string
return commonPrefix != null && commonPrefix !== '' ? commonPrefix : null
}
/** Generate a name for a project using given base string. A suffix is added if there is a
@ -295,7 +291,7 @@ interface NameAndPath {
* If given a name like `Name_1` it will become `Name_2` if there is already a directory named
* `Name_1`. If a path containing multiple components is given, only the last component is used
* for the name. */
export function generateDirectoryName(name: string): NameAndPath {
export function generateDirectoryName(name: string): string {
// Use only the last path component.
name = pathModule.parse(name).name
@ -311,16 +307,17 @@ export function generateDirectoryName(name: string): NameAndPath {
}
const projectsDirectory = getProjectsDirectory()
let finalPath: string
while (true) {
suffix++
const newName = `${name}${suffix === 0 ? '' : `_${suffix}`}`
const candidatePath = pathModule.join(projectsDirectory, newName)
if (!fs.existsSync(candidatePath)) {
// eslint-disable-next-line no-restricted-syntax
return { name: newName, path: candidatePath }
finalPath = candidatePath
break
}
}
// Unreachable.
return finalPath
}
/** Take a path to a file, presumably located in a project's subtree.Returns the path
@ -353,33 +350,6 @@ export function isProjectInstalled(projectRoot: string): boolean {
return pathModule.resolve(projectRootParent) === pathModule.resolve(projectsDirectory)
}
/** Get the name of the project from the bundle metadata (`package.yaml`). */
export function getProjectName(projectRoot: string) {
const metadataPath = pathModule.join(projectRoot, paths.BUNDLE_METADATA_RELATIVE)
const metadata = fs.readFileSync(metadataPath, 'utf-8')
return metadata.match(/^name: (.+)$/)?.[1] ?? ''
}
/** Update the name of the project in the bundle metadata (`package.yaml`). */
export function updateName(projectRoot: string, newName: string) {
const metadataPath = pathModule.join(projectRoot, paths.BUNDLE_METADATA_RELATIVE)
const oldMetadata = fs.readFileSync(metadataPath, 'utf-8')
const newMetadata = oldMetadata.replace(/^name: .+$/m, `name: ${newName}`)
fs.writeFileSync(metadataPath, newMetadata)
}
/** Gets the name of a bundle from the bundle metadata (`package.yaml`). */
export function tryGetName(projectRoot: string) {
const metadataPath = pathModule.join(projectRoot, paths.BUNDLE_METADATA_RELATIVE)
try {
const metadata = fs.readFileSync(metadataPath, 'utf-8')
return metadata.match(/^name: (.+)$/m)?.[1] ?? null
} catch {
/// The bundle metadata file was not found.
return null
}
}
// ==================
// === Project ID ===
// ==================

View File

@ -99,10 +99,14 @@ export default function DriveView(props: DriveViewProps) {
const setAssets = React.useCallback(
(newAssets: backendModule.AnyAsset[]) => {
rawSetAssets(newAssets)
// The project name here might also be a string with project id, e.g. when opening
// a project file from explorer on Windows.
const isInitialProject = (asset: backendModule.AnyAsset) =>
asset.title === initialProjectName || asset.id === initialProjectName
if (nameOfProjectToImmediatelyOpen != null) {
const projectToLoad = newAssets
.filter(backendModule.assetIsProject)
.find(projectAsset => projectAsset.title === nameOfProjectToImmediatelyOpen)
.find(isInitialProject)
if (projectToLoad != null) {
dispatchAssetEvent({
type: assetEventModule.AssetEventType.openProject,
@ -111,12 +115,14 @@ export default function DriveView(props: DriveViewProps) {
}
setNameOfProjectToImmediatelyOpen(null)
}
if (!initialized && initialProjectName != null) {
if (!initialized) {
setInitialized(true)
if (!newAssets.some(asset => asset.title === initialProjectName)) {
const errorMessage = `No project named '${initialProjectName}' was found.`
toastify.toast.error(errorMessage)
logger.error(`Error opening project on startup: ${errorMessage}`)
if (initialProjectName != null) {
if (!newAssets.some(isInitialProject)) {
const errorMessage = `No project named '${initialProjectName}' was found.`
toastify.toast.error(errorMessage)
logger.error(`Error opening project on startup: ${errorMessage}`)
}
}
}
},
@ -131,8 +137,10 @@ export default function DriveView(props: DriveViewProps) {
)
React.useEffect(() => {
setAssets([])
// `setAssets` is a callback, not a dependency.
if (initialized) {
setAssets([])
}
// `setAssets` is a callback, not a dependency. `initialized` is not a dependency either.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [backend])

View File

@ -118,19 +118,14 @@ export default function ProjectNameColumn(props: ProjectNameColumnProps) {
rowState.setPresence(presence.Presence.inserting)
try {
if (backend.type === backendModule.BackendType.local) {
/** Information required to display a bundle. */
interface BundleInfo {
name: string
id: string
}
// This non-standard property is defined in Electron.
let info: BundleInfo
let id: string
if (
'backendApi' in window &&
// This non-standard property is defined in Electron.
'path' in file &&
typeof file.path === 'string'
) {
info = await window.backendApi.importProjectFromPath(file.path)
id = await window.backendApi.importProjectFromPath(file.path)
} else {
const response = await fetch('./api/upload-project', {
method: 'POST',
@ -140,15 +135,17 @@ export default function ProjectNameColumn(props: ProjectNameColumnProps) {
// work on `http://localhost`.
body: await file.arrayBuffer(),
})
// This is SAFE, as the types of this API are statically known.
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
info = await response.json()
id = await response.text()
}
const listedProject = await backend.getProjectDetails(
backendModule.ProjectId(id),
null
)
rowState.setPresence(presence.Presence.present)
setItem({
...item,
title: info.name,
id: backendModule.ProjectId(info.id),
title: listedProject.packageName,
id: backendModule.ProjectId(id),
})
} else {
const fileName = item.title

View File

@ -23,17 +23,11 @@ interface Enso {
// === Backend API ===
// ===================
/** Information required to display a bundle. */
interface BundleInfo {
name: string
id: string
}
/** `window.backendApi` is a context bridge to the main process, when we're running in an
* Electron context. It contains non-authentication-related functionality. */
interface BackendApi {
/** Return the ID of the new project. */
importProjectFromPath: (openedPath: string) => Promise<BundleInfo>
importProjectFromPath: (openedPath: string) => Promise<string>
}
// ==========================