feat: Extending linux executableArgs option to be utilized for Snap target (fixes #4587)

[fangpenlin]

Fix #4587

The executableArgs option value is ignored for snap target, this PR fixes that bug

[changeset-bot]

🦋 Changeset detected

Latest commit: 7bf0f81

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages

Name	Type
app-builder-lib	Major
dmg-builder	Major
electron-builder-squirrel-windows	Major
electron-builder	Major
electron-forge-maker-appimage	Major
electron-forge-maker-nsis-web	Major
electron-forge-maker-nsis	Major
electron-forge-maker-snap	Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[netlify]

✅ Deploy Preview for car-park-attendant-cleat-11576 ready!

Name	Link
🔨 Latest commit	7bf0f81
🔍 Latest deploy log	https://app.netlify.com/sites/car-park-attendant-cleat-11576/deploys/6351ce5bf55d9c00093a97f3
😎 Deploy Preview	https://deploy-preview-7198--car-park-attendant-cleat-11576.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[mmaietta]

If extraAppArgs = ['--test', '--test2'], then this would fail, right?
It would be --extraAppArgs=--test --test2. Do we need to wrap it in double quotes? (i.e. --extraAppArgs="--test --test2 --no-sandbox")

[mmaietta]

I would simplify this to be:

const executableArgs: string[] = options.executableArgs ?? []
if (this.isElectronVersionGreaterOrEqualThan("5.0.0") && !isBrowserSandboxAllowed(snap)) {
  executableArgs.push('--no-sandbox')
  if (this.isUseTemplateApp) {
    args.push("--exclude", "chrome-sandbox")
  } 
}
const extraArgs = new Array(new Set(executableArgs)) // remove all duplicate entries
if (extraArgs.length) {
  args.push(`--extraAppArgs="${extraArgs.join(" ")}"`)  
}

[fangpenlin]

I tested it with

executableArgs: ["--no-sandbox", "--disable-seccomp-filter-sandbox"]

for solving a problem of my app similar to this one here and it's working as expected for my app. The args are passed to underlaying os execv-like system call as string array as the argument instead of a piece of shell argument string, the app-builder-bin shall receive the value as is, so we probably don't need to escape the value there

for removing duplicate arguments, personally I would prefer to lean toward more of keeping the argument as is, because

• the order of argument might matter, like ["--run-a", "--run-b"] and ["--run-b", "--run-a"] might have different implication
• repeating argument might be meaningful for some cases, such as ["--verbose", "--verbose", "--verbose"], while this might not be the best example and best argument design in the world, there might still be some legit use cases where you want to pass repeating argument, like you want to test your electron argument handler and ensure when seeing duplicate argument it throws error

[mmaietta]

Very interesting and great reasoning. I like it 👍

[mmaietta]

Quick note: Since this could impact all linux builds with executableArgs set and this was never supported for Snap, I would consider this more of a feature or breaking change. (Luckily we have v24-alpha en route so we can include this there as a Breaking Change just to play it safe)