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.

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

Create profile directory if it doesn't exist and --profile-create-new has been combined to the -p cli option #2058

Closed
blimmer opened this issue Oct 26, 2020 · 10 comments · Fixed by #2064

Comments

@blimmer
Copy link

blimmer commented Oct 26, 2020

Is this a feature request or a bug?

Feature request

What is the current behavior?

Today, if I pass a profile that does not exist, the web-ext run command fails:

> web-ext run --source-dir ./dist/ -p extension_development
Applying config file: ./package.json
Running web extension from /Users/a70e7cc5/code/proj/dist

o: Could not copy Firefox profile from extension_development: Error: cannot find profile extension_development
    at Module.C (/Users/a70e7cc5/code/proj/node_modules/web-ext/dist/web-ext.js:1:22412)

What is the expected or desired behavior?

It would be nice if there was a flag to create the profile if it doesn't exist. Even better, it would be nice to be able to pass a directory to create the profile in.

I'm able to work around this in my launch script:

import webExt from 'web-ext';
import { default as fxRunner } from 'fx-runner';

async function launchFirefox(browserProfileDirectory: string) {
  await createFirefoxProfileIfNotExists(browserProfileDirectory);
  await webExt.cmd.run({
    sourceDir: BUILD_DIR,
    firefoxProfile: browserProfileDirectory,
    keepProfileChanges: true,
    startUrl: ['about:debugging#/runtime/this-firefox'],
  });
}

async function createFirefoxProfileIfNotExists(browserProfileDirectory: string): Promise<void> {
  const devProfileName = 'extension_development';

  if (!fs.existsSync(browserProfileDirectory)) {
    console.info(`${devProfileName} Firefox profile does not exist. Creating...`);

    // The `-CreateProfile` flag creates a new Firefox profile stub and immediately exits
    const createProfile = await fxRunner({
      'no-remote': true,
      'binary-args': ['-CreateProfile', `${devProfileName} ${browserProfileDirectory}`],
    });
    const createProfileProc = createProfile.process;
    return new Promise((resolve, reject) => {
      createProfileProc.on('close', resolve);
      createProfileProc.on('error', reject);
    });
  }
}

Version information (for bug reports)

  • Firefox version: 83.0b4
  • Your OS and version: MacOS 10.15.7
  • Paste the output of these commands:
> node --version && npm --version && web-ext --version
v12.18.4
6.14.6
5.3.0
@rpl
Copy link
Member

rpl commented Oct 27, 2020

Having this behavior as an optional opt-in feature, e.g. enabled by a new --profile-create-new, sounds reasonable to me too.

@rpl
Copy link
Member

rpl commented Oct 27, 2020

mentor: @rpl

To fix this issue we want to:

  • define a new command line option for the web-ext run command
  • when the new --profile-create-new option is specified:
    • the value set on --profile is going to be considered as a directory path
    • if the directory already exit, exit immediately, show to the user an error message to warn the user
    • if the directory doesn't exit, create it and use it as the firefox profile for the extension runner
  • cover the new expected behavior with new unit tests

We should also double-check the behavior of the similar --chrome-profile option, because it may be reasonable to also behave consistently when it is combined with the --profile-create-new cli option.

Follows some of the web-ext source code that may be interested by this change:

  • The web-ext command line options are all defined in src/program.js (
    .command('run', 'Run the extension', commands.run, {
    )
  • The web-ext run command is implemented in src/cmd/run.js (
    export default async function run(
    )
  • Existing internal helper methods used to find and manage the Firefox profile are defined in src/firefox/index.js
  • Existing tests for the src/cmd/run.js module are defined in tests/unit/test-cmd/test.run.js

@rpl rpl changed the title CreateProfile if it doesn't exist for web-ext run Create profile directory if it doesn't exist and --profile-create-new has been combined to the -p cli option Oct 27, 2020
@akhilpanchal
Copy link
Contributor

akhilpanchal commented Oct 29, 2020

Hi @rpl
I am interested in working on this. This is my first time contributing to this project, so I might need some time to familiarize myself with the project. I shall ask if I have any questions or face any issues.

@rpl
Copy link
Member

rpl commented Oct 29, 2020

Hi @rpl
I am interested in working on this. This is my first time contributing to this project, so I might need some time to familiarize myself with the project. I shall ask if I have any questions or face any issues.

@akhilpanchal Sure thing!

Given that this would be your first contribution, I suggest you to also:

After that you should make sure to be able to reproduce (and then understand) the issue described in this issue, and then look to the pointers provided in #2058.

Feel free to ask for help if you have any specific questions or issues.

@ankushduacodes
Copy link
Contributor

@rpl is it okay if I work on this issue as well?

@akhilpanchal
Copy link
Contributor

@ankushduacodes I have already started working on this issue. I should have a PR soon.

@akhilpanchal
Copy link
Contributor

akhilpanchal commented Nov 2, 2020

@rpl
I am able to reproduce the issue.
However I am a little unsure if I understand the role of this new option.

Is the expectation to create a directory and also create a new Firefox Profile in this directory?
OR
is the expectation to just create a directory if it the given directory doesn't exist so that web-ext run can proceed without complaining that the profile directory doesn't exist?

@blimmer
Copy link
Author

blimmer commented Nov 2, 2020

For my use case, the latter would be preferable. The profile gets created if it doesn't already exist.

@rpl
Copy link
Member

rpl commented Nov 2, 2020

@rpl
I am able to reproduce the issue.
However I am a little unsure if I understand the role of this new option.

Is the expectation to create a directory and also create a new Firefox Profile in this directory?
OR
is the expectation to just create a directory if it the given directory doesn't exist so that web-ext run can proceed without complaining that the profile directory doesn't exist?

@akhilpanchal
👍 I think that (when the new option is being used) we should just have to create that directory if it doesn't exist already, everything else should work as expected without any additional special handling.

Let's start with doing that (and create a new test case to cover those changes), as part of the pull request review we will also double-check (while looking to the actual changes) if there is anything more that should be done.

@blimmer
Copy link
Author

blimmer commented Nov 23, 2020

I saw this got released with 5.4.0 and just started using it. Thank you to everyone who helped out with this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants