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
fix(launcher): support Firefox as a snap #21328
Changes from all commits
f0fd6de
9a38223
3694f71
eea4eb2
cbda09a
918042e
5e83292
a81493d
4d8aae3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,8 +3,36 @@ import type { FoundBrowser, Browser, PathData } from '../types' | |
import { notInstalledErr } from '../errors' | ||
import { utils } from '../utils' | ||
import os from 'os' | ||
import { promises as fs } from 'fs' | ||
import path from 'path' | ||
import Bluebird from 'bluebird' | ||
import which from 'which' | ||
|
||
async function isFirefoxSnap (binary: string): Promise<boolean> { | ||
try { | ||
return await Bluebird.resolve((async () => { | ||
const binaryPath = await which(binary) | ||
|
||
// if the bin path or what it's symlinked to start with `/snap/bin`, it's a snap | ||
if (binaryPath.startsWith('/snap/bin/') || (await fs.realpath(binaryPath)).startsWith('/snap/bin')) return true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🥳 |
||
|
||
// read the first 16kb, don't read the entire file into memory in case it is a binary | ||
const fd = await fs.open(binaryPath, 'r') | ||
// @ts-ignore - needs @types/node at least 16 | ||
// https://github.com/cypress-io/cypress/issues/21329 | ||
const { buffer, bytesRead } = await fd.read<Buffer>({ length: 16384 }) | ||
|
||
await fd.close() | ||
|
||
return buffer.slice(0, bytesRead).toString('utf8').includes('exec /snap/bin/firefox') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Today I learned about reading only the opening bytes of a file! This seemed odd (particularly the slice), so I poked around the docs and command line, and lo, you have done it correctly. :) |
||
})()) | ||
.timeout(30000) | ||
} catch (err) { | ||
log('failed to check if Firefox is a snap, assuming it isn\'t %o', { err, binary }) | ||
|
||
return false | ||
} | ||
} | ||
|
||
function getLinuxBrowser ( | ||
name: string, | ||
|
@@ -43,11 +71,25 @@ function getLinuxBrowser ( | |
throw notInstalledErr(binary) | ||
} | ||
|
||
const maybeSetSnapProfilePath = (versionString: string) => { | ||
if (os.platform() === 'linux' && name === 'chromium' && versionString.endsWith('snap')) { | ||
const maybeSetSnapProfilePath = async (versionString: string) => { | ||
if (os.platform() !== 'linux') return | ||
|
||
if (name === 'chromium' && versionString.endsWith('snap')) { | ||
// when running as a snap, chromium can only write to certain directories | ||
// @see https://github.com/cypress-io/cypress/issues/7020 | ||
log('chromium is running as a snap, changing profile path') | ||
foundBrowser.profilePath = path.join(os.homedir(), 'snap', 'chromium', 'current') | ||
|
||
return | ||
} | ||
|
||
if (name === 'firefox' && (await isFirefoxSnap(binary))) { | ||
// if the binary in the path points to a script that calls the snap, set a snap-specific profile path | ||
// @see https://github.com/cypress-io/cypress/issues/19793 | ||
log('firefox is running as a snap, changing profile path') | ||
foundBrowser.profilePath = path.join(os.homedir(), 'snap', 'firefox', 'current') | ||
|
||
return | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to avoid Bluebird promises in newly written code when possible. Especially since we're already inside an
async
function, do we needBluebird.resolve
here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using Bluebird.resolve to get access to
.timeout
. I couldPromise.race/new Promise/setTimeout
but I figure that unless we are planning on moving all the code in the code base to not use Bluebird, might as well use the utilities we have. I also generally agree that BB should be avoided in new code.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is something I would like to do at some point, remove bluebird from our existing code. But adding timeout utility functions is beyond the scope of this PR, fair enough.