Skip to content

[Fix] Make checkDiskSpace look at the correct drive again #3654

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

Merged
merged 2 commits into from
Mar 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 1 addition & 7 deletions src/backend/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -563,16 +563,10 @@ ipcMain.on('unlock', () => {
})

ipcMain.handle('checkDiskSpace', async (_e, folder): Promise<DiskSpaceData> => {
// We only need to look at the root directory for used/free space
// Trying to query this for a directory that doesn't exist (which `folder`
// might be) will not work
const { root } = path.parse(folder)

// FIXME: Propagate errors
const parsedPath = Path.parse(folder)
const parsedRootPath = Path.parse(root)

const { freeSpace, totalSpace } = await getDiskInfo(parsedRootPath)
const { freeSpace, totalSpace } = await getDiskInfo(parsedPath)
const pathIsWritable = await isWritable(parsedPath)
const pathIsFlatpakAccessible = isAccessibleWithinFlatpakSandbox(parsedPath)

Expand Down
59 changes: 59 additions & 0 deletions src/backend/utils/filesystem/__tests__/unix.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import fs from 'fs'
import * as util_os_processes from '../../os/processes'
import { getDiskInfo_unix } from '../unix'
import type { Path } from 'backend/schemas'

describe('getDiskInfo_unix', () => {
it('Works with root path', async () => {
const accessSyp = jest
.spyOn(fs.promises, 'access')
.mockImplementation(async () => Promise.resolve())
const spawnWrapperSpy = jest
.spyOn(util_os_processes, 'genericSpawnWrapper')
.mockImplementation(async () => {
return Promise.resolve({
stdout:
'Filesystem 1024-blocks Used Available Capacity Mounted on\n/dev/sda1 100 90 10 90% /',
stderr: '',
exitCode: null,
signalName: null
})
})

const ret = await getDiskInfo_unix('/' as Path)
expect(ret.totalSpace).toBe(100 * 1024)
expect(ret.freeSpace).toBe(10 * 1024)
expect(accessSyp).toHaveBeenCalledTimes(1)
expect(spawnWrapperSpy).toHaveBeenCalledWith('df', ['-P', '-k', '/'])
expect(spawnWrapperSpy).toHaveBeenCalledTimes(1)
})

it('Works with nested path', async () => {
const accessSyp = jest
.spyOn(fs.promises, 'access')
.mockImplementation(async (path) => {
if (path === '/foo/bar/baz') {
return Promise.reject()
}
return Promise.resolve()
})
const spawnWrapperSpy = jest
.spyOn(util_os_processes, 'genericSpawnWrapper')
.mockImplementation(async () => {
return Promise.resolve({
stdout:
'Filesystem 1024-blocks Used Available Capacity Mounted on\n/dev/sda1 100 90 10 90% /foo/bar',
stderr: '',
exitCode: null,
signalName: null
})
})

const ret = await getDiskInfo_unix('/foo/bar/baz' as Path)
expect(ret.totalSpace).toBe(100 * 1024)
expect(ret.freeSpace).toBe(10 * 1024)
expect(accessSyp).toHaveBeenCalledTimes(2)
expect(spawnWrapperSpy).toHaveBeenCalledWith('df', ['-P', '-k', '/foo/bar'])
expect(spawnWrapperSpy).toHaveBeenCalledTimes(1)
})
})
67 changes: 67 additions & 0 deletions src/backend/utils/filesystem/__tests__/windows.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import * as util_os_processes from '../../os/processes'
import { getDiskInfo_windows } from '../windows'
import type { Path } from 'backend/schemas'

describe('getDiskInfo_windows', () => {
it('Works with root path', async () => {
const spawnWrapperSpy = jest
.spyOn(util_os_processes, 'genericSpawnWrapper')
.mockImplementation(async () => {
return Promise.resolve({
stdout: JSON.stringify([{ Caption: 'C:', FreeSpace: 10, Size: 100 }]),
stderr: '',
exitCode: null,
signalName: null
})
})

const ret = await getDiskInfo_windows('C:' as Path)
expect(ret.totalSpace).toBe(100)
expect(ret.freeSpace).toBe(10)
expect(spawnWrapperSpy).toHaveBeenCalledWith('powershell', [
'Get-CimInstance',
'-Class',
'Win32_LogicalDisk',
'-Property',
'Caption,FreeSpace,Size',
'|',
'Select-Object',
'Caption,FreeSpace,Size',
'|',
'ConvertTo-Json',
'-Compress'
])
expect(spawnWrapperSpy).toHaveBeenCalledTimes(1)
})

it('Works with nested path', async () => {
const spawnWrapperSpy = jest
.spyOn(util_os_processes, 'genericSpawnWrapper')
.mockImplementation(async () => {
return Promise.resolve({
stdout: JSON.stringify([{ Caption: 'C:', FreeSpace: 10, Size: 100 }]),
stderr: '',
exitCode: null,
signalName: null
})
})

const ret = await getDiskInfo_windows('C:/foo/bar/baz' as Path)
expect(ret.totalSpace).toBe(100)
expect(ret.freeSpace).toBe(10)
expect(spawnWrapperSpy).toHaveBeenCalledWith('powershell', [
'Get-CimInstance',
'-Class',
'Win32_LogicalDisk',
'-Property',
'Caption,FreeSpace,Size',
'|',
'Select-Object',
'Caption,FreeSpace,Size',
'|',
'ConvertTo-Json',
'-Compress'
])
expect(spawnWrapperSpy).toHaveBeenCalledTimes(1)
})
})
4 changes: 4 additions & 0 deletions src/backend/utils/filesystem/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ interface DiskInfo {
totalSpace: number
}

/**
* Gathers information about the disk `path` is on.
* `path` does not have to exist.
*/
async function getDiskInfo(path: Path): Promise<DiskInfo> {
switch (process.platform) {
case 'linux':
Expand Down
19 changes: 18 additions & 1 deletion src/backend/utils/filesystem/unix.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import { genericSpawnWrapper } from '../os/processes'
import { access } from 'fs/promises'
import { join } from 'path'

import type { Path } from 'backend/schemas'
import type { DiskInfo } from './index'

async function getDiskInfo_unix(path: Path): Promise<DiskInfo> {
const { stdout } = await genericSpawnWrapper('df', ['-P', '-k', path])
const rootPath = await findFirstExistingPath(path)

const { stdout } = await genericSpawnWrapper('df', ['-P', '-k', rootPath])
const lineSplit = stdout.split('\n')[1].split(/\s+/)
const [, totalSpaceKiBStr, , freeSpaceKiBStr] = lineSplit
return {
Expand All @@ -14,6 +17,20 @@ async function getDiskInfo_unix(path: Path): Promise<DiskInfo> {
}
}

/**
* Finds the first existing path in the path's hierarchy
* @example
* findFirstExistingPath('/foo/bar/baz')
* // => '/foo/bar/baz' if it exists, otherwise '/foo/bar', otherwise '/foo', otherwise '/'
*/
async function findFirstExistingPath(path: Path): Promise<Path> {
let maybeExistingPath = path
while (!(await isWritable_unix(maybeExistingPath))) {
maybeExistingPath = join(maybeExistingPath, '..') as Path
}
return maybeExistingPath
}

async function isWritable_unix(path: Path): Promise<boolean> {
return access(path).then(
() => true,
Expand Down
Loading