mozilla / web-ext

A command line tool to help build, run, and test web extensions
Mozilla Public License 2.0
2.7k stars 338 forks source link

Tool seems to assume source-dir has git commit if it contains .git directory #1349

Open Gozala opened 6 years ago

Gozala commented 6 years ago

Is this a feature request or a bug?

This is a bug

What is the current behavior?

If you start a web-extension and did git init but have not yet committed any changes this seems to trigger a bug in the implementation causing a crash. Here is the trace:

MOZ_DISABLE_CONTENT_SANDBOX=1 web-ext run --firefox=nightly

(node:19028) UnhandledPromiseRejectionWarning: Error: ENOENT: no such file or directory,open '/Users/gozala/Projects/libdweb-template/.git/packed-refs'
    at Object.fs.openSync (fs.js:663:18)
    at Object.fs.readFileSync (fs.js:568:33)
    at Object.long (/Users/gozala/Projects/libdweb-template/node_modules/git-rev-sync/index.js:109:31)
    at defaultVersionGetter (/Users/gozala/Projects/libdweb-template/node_modules/web-ext/dist/webpack:/src/program.js:266:53)
    at Program._callee$ (/Users/gozala/Projects/libdweb-template/node_modules/web-ext/dist/webpack:/src/program.js:159:21)
    at tryCatch (/Users/gozala/Projects/libdweb-template/node_modules/regenerator-runtime/runtime.js:62:40)    at Generator.invoke [as _invoke] (/Users/gozala/Projects/libdweb-template/node_modules/regenerator-runtime/runtime.js:296:22)
    at Generator.prototype.(anonymous function) [as next] (/Users/gozala/Projects/libdweb-template/node_modules/regenerator-runtime/runtime.js:114:21)
    at step (/Users/gozala/Projects/libdweb-template/node_modules/web-ext/dist/webpack:/node_modules/babel-runtime/helpers/asyncToGenerator.js:17:1)
    at /Users/gozala/Projects/libdweb-template/node_modules/web-ext/dist/webpack:/node_modules/babel-runtime/helpers/asyncToGenerator.js:35:1
(node:19028) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:19028) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js processwith a non-zero exit code.

If I create a commit this issue no longer occurs, neither it does if git init has not being run. It is also worth noting that .git/packed-refs does not exists after first commit as well, but presumably something else does causing differ code path.

What is the expected or desired behavior?

web-ext run should behave same regardless of git repo state

Version information (for bug reports)

node --version && npm --version && web-ext --version
v9.5.0
5.6.0
master-065188db35a4f63c2d17ff744cc1de2601a62222
rpl commented 6 years ago

@Gozala I'm not able to reproduce this locally yet.

is it an issue reproducible from a custom build? Looking at the following code from function that seems to be triggering it (from the stack trace attached above):

https://github.com/mozilla/web-ext/blob/daf1d715ae3b2ca28840fdcd3b30ad87f3216213/src/program.js#L255-L268

a production build should always just take the version from the web-ext package.json file, and it should never call the git.branch and git.long methods.

Do you mind to provide some additional context/STR? (e.g. some details about how the web-ext package has been built and then installed or linked as a dev dependency to trigger this issue).

Gozala commented 6 years ago

@rpl I end up running into this when I tried to add web-ext as git dev-dependency. I did run npm install && npm run bulid postinstall.

Next week is going to be really busy for me, but I'll be able to look more into it to provide more context after I'm back from summit week after.

Rob--W commented 6 years ago

@Gozala Can you write the exact steps of how we can reproduce the problem? I tried adding web-ext pulling in mozilla/web-ext as a devDependency, and can't even get npm run build to work. Here are the steps that I took:

  1. Create a package.json file with npm init
  2. Install and save as git devDependency using npm install --save-dev mozilla/web-ext (version: 8f03dfde6a8f72a2913c891b2d989a29b7560d83).
  3. cd node_modules/web-ext
  4. npm install to install dependencies.
  5. npm run build (which calls grunt build).

Result (node: this seems related to the "web-ext" directory being inside a node_modules directory; if I rename it to nod_modules web-ext will build just fine, but then running web-ext/bin/web-ext run completes successfully.

Running "clean:dist" (clean) task
>> 1 path cleaned.

Running "webpack:build" (webpack) task
Hash: 0a6be0a291553ba1ec89                                                         
Version: webpack 3.11.0
Time: 54ms
         Asset     Size  Chunks             Chunk Names
    web-ext.js  4.56 kB       0  [emitted]  main
web-ext.js.map  2.92 kB       0  [emitted]  main
   [0] ./src/main.js 239 bytes {0} [built]
   [1] ./src/program.js 202 bytes {0} [built] [failed] [1 error]
   [2] ./src/cmd/index.js 243 bytes {0} [built] [failed] [1 error]
   [3] ./src/util/logger.js 241 bytes {0} [built] [failed] [1 error]

ERROR in ./src/program.js
Module parse failed: Unexpected token (25:5)
You may need an appropriate loader to handle this file type.
| 
| 
| type ProgramOptions = {|
|   absolutePackageDir?: string,
| |}
 @ ./src/main.js 2:0-31

ERROR in ./src/cmd/index.js
Module parse failed: Unexpected token (3:12)
You may need an appropriate loader to handle this file type.
| /* @flow */
| 
| import type {
|   BuildCmdParams, BuildCmdOptions, ExtensionBuildResult,
| } from './build';
 @ ./src/main.js 3:0-24

ERROR in ./src/util/logger.js
Module parse failed: Unexpected token (8:7)
You may need an appropriate loader to handle this file type.
| // Bunyan-related Flow types
| 
| export type TRACE = 10;
| export type DEBUG = 20;
| export type INFO = 30;
 @ ./src/main.js 4:0-40
Warning:  Use --force to continue.

Aborted due to warnings.
l0b0 commented 10 months ago

Reproducible build + test of what I assume is the equivalent bug in 7.6.2 (most of the commit is a workaround for #2981):

❯ nix-build -A web-ext.passthru.tests
this derivation will be built:
  /nix/store/x88rykyx82j5k8942z8mymd56ncrrszs-web-ext-tests.drv
building '/nix/store/x88rykyx82j5k8942z8mymd56ncrrszs-web-ext-tests.drv'...
Initialized empty Git repository in /build/.git/
[main (root-commit) 8f95cbc] empty commit
/nix/store/ns3qqxx2r7j3x1fdrbn12gg9w91xmiy5-web-ext-7.6.2/lib/node_modules/web-ext/node_modules/git-rev-sync/index.js:47
    throw new Error('[git-rev-sync] no git repository found');
          ^

Error: [git-rev-sync] no git repository found
    at _getGitDirectory (/nix/store/ns3qqxx2r7j3x1fdrbn12gg9w91xmiy5-web-ext-7.6.2/lib/node_modules/web-ext/node_modules/git-rev-sync/index.js:47:11)
    at _getGitDirectory (/nix/store/ns3qqxx2r7j3x1fdrbn12gg9w91xmiy5-web-ext-7.6.2/lib/node_modules/web-ext/node_modules/git-rev-sync/index.js:72:10)
    at _getGitDirectory (/nix/store/ns3qqxx2r7j3x1fdrbn12gg9w91xmiy5-web-ext-7.6.2/lib/node_modules/web-ext/node_modules/git-rev-sync/index.js:72:10)
    at _getGitDirectory (/nix/store/ns3qqxx2r7j3x1fdrbn12gg9w91xmiy5-web-ext-7.6.2/lib/node_modules/web-ext/node_modules/git-rev-sync/index.js:72:10)
    at _getGitDirectory (/nix/store/ns3qqxx2r7j3x1fdrbn12gg9w91xmiy5-web-ext-7.6.2/lib/node_modules/web-ext/node_modules/git-rev-sync/index.js:72:10)
    at _getGitDirectory (/nix/store/ns3qqxx2r7j3x1fdrbn12gg9w91xmiy5-web-ext-7.6.2/lib/node_modules/web-ext/node_modules/git-rev-sync/index.js:72:10)
    at _getGitDirectory (/nix/store/ns3qqxx2r7j3x1fdrbn12gg9w91xmiy5-web-ext-7.6.2/lib/node_modules/web-ext/node_modules/git-rev-sync/index.js:72:10)
    at Module.branch (/nix/store/ns3qqxx2r7j3x1fdrbn12gg9w91xmiy5-web-ext-7.6.2/lib/node_modules/web-ext/node_modules/git-rev-sync/index.js:76:16)
    at defaultVersionGetter (file:///nix/store/ns3qqxx2r7j3x1fdrbn12gg9w91xmiy5-web-ext-7.6.2/lib/node_modules/web-ext/lib/program.js:303:19)
    at async Object.main (file:///nix/store/ns3qqxx2r7j3x1fdrbn12gg9w91xmiy5-web-ext-7.6.2/lib/node_modules/web-ext/lib/program.js:326:19)

Node.js v18.18.2
Rob--W commented 10 months ago

@l0b0 Adding git-rev-sync is not the solution here. You should build with the --production flag set if the intent is to mimic the actual build procedure.

Note: make sure to build from a specific tag, not a random unreleased commit.

l0b0 commented 10 months ago

@Rob--W web-ext doesn't run at all without git-rev-sync installed.

l0b0 commented 10 months ago

Note: make sure to build from a specific tag, not a random unreleased commit.

I'm running from the 7.6.2 tag.