brave-browser: Remove `deep-freeze-node` dependency: the project is no longer installable on mac arm

I can’t install the project. Steps to reproduce the issue:

git clone git@github.com:brave/brave-browser.git
cd brave-browser
npm install
cd src
rm -rf brave # otherwise error "directory already exists", BTW that should be mentioned in the doc
cd ..
npm run init

And I get:

    fatal: repository 'undefined' does not exist

It seems the function getNPMConfig relies on env vars that are obviously not present on my system. But there is no mention about it in the Brave installation procedure.

What’s wrong here?

Just in case, I mention that I’m on macOS 10.14.6 (Mojave).

About this issue

  • Original URL
  • State: closed
  • Created 3 years ago
  • Comments: 52 (10 by maintainers)

Most upvoted comments

I also encountered the same issue. Problem is that the path to the repository url is broken.

There is a string replacement that is making - into _ at https://github.com/brave/brave-browser/blob/master/lib/util.js#L5

So when brave-browser goes looking for brave-core in package.json, it can never find it because the key is changed to brave_core and it ends up undefined

You can change it to brave_core in https://github.com/brave/brave-browser/blob/master/package.json#L38

That fixes it there, but then it breaks in brave-core which has its own getNPMConfig that does not do the same string replacement to change -. So it scans for brave-core but never finds it because it is brave_core now.

You can then change at https://github.com/brave/brave-core/blob/master/build/commands/lib/config.js#L77 brave-core to brave_core so when it scans for the repo url it can find it because it is now brave_core in brave-browser/package.json.

Then change the repo url and branch name to point to the brave-core version that has this change:

Screen Shot 2021-01-19 at 8 45 30 AM

I was able to install using these steps, but it’s a patch job and should probably have a real fix instead.

I agree that all the setup instructions should be together so we’ll reorganize things a little bit

On Feb 22, 2021, at 12:14 AM, Brian Clifton notifications@github.com wrote:

@Kwadz there’s only a different procedure if you make edits to the code. Everyone has read access to the repo, but creating branches on the remote and pushing to the remote is limited to employees

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

@jtrag if you are experiencing this problem you are using the wrong version of npm - https://github.com/brave/brave-browser/issues/13631#issuecomment-797611763

@Aragar199 you’re going to want to install the LTS version of Node.js, which should be 14.x LTS requirement captured here: https://github.com/brave/brave-browser/wiki/macOS-Development-Environment

@bsclifton I reopened because there is still an outstanding issue with the npm config

Tested with node v14.15.5 here (latest LTS), and it works. @bsclifton was right from the beginning and I also missed it 🤦‍♂️

Thank you guys!

yep, npm 6.x should work correctly because that is what I have locally

@bsclifton the problem is likely the npm version because I believe that is what converts the config to env vars, but presumably downgrading to node 14 will also take care of npm. There must be a change in the format when they are converted to env vars

Yep, that’s it! I installed node 12 and it works fine. Still throws an error if the directory src/brave/ already exists due to a failed installation but it’s much more verbose so I’m inclined to just nuke my PR and call it. My bad, @bsclifton, you mentioned this near the beginning of this thread!

I think the documentation should be clarified.

However- if you’re actively making changes in a fork, you’ll need to add YOUR remote to the src/brave repo. Only Brave employees have access to push to brave/brave-core - so you’ll need to push to your fork if you’re hoping to submit a pull request. I hope that makes sense. If you’re not needing to submit a PR, you can skip the Making changes section

You seem to tell the section Clone and initialize the repo is for Brave employees and the section Making change for contributors.

If there is a different procedure for Brave employees and a for contributors, that should be mentioned.

Basically, all you need to do is run npm run init and src/brave will be setup for you.

This npm run init is not even mentioned in the section Making change. I think all the steps of the procedure should be mentioned in this section, or at least with a link to the proper section to build the project.

My two cents about the doc:

For example in the section Clone and initialize the repo, “Clone and initialize the repo” for what? It’s not clear that part is not for contributing, may be we should mention something not to confuse with this section.

Also, the section Making change does not mention anything about npm run init. So, it’s not complete and the reader think we should go to Clone and initialize the repo to continue the setup.

I’d also update in Making change:

Once you’ve cloned the repo to your computer, you’re ready to start making edits!

to

Once you’ve cloned your fork to your computer, you’re ready to start making edits!

Hope my new eyes can help to clarify the doc and therefore make it easier for new contributors.

Not sure where that came from cc @bsclifton, I’ll check out the PR change and make sure all the docs are current either this weekend or first thing Monday

On Feb 20, 2021, at 8:28 AM, Kwadz notifications@github.com wrote:

Thanks @bridiver, in that case I think this section of the documentation should be updated:

git clone git@github.com:brave/brave-browser.git cd brave-browser npm install

this takes 30-45 minutes to run

the Chromium source is downloaded which has a large history

npm run init It does not even match with this section:

root where project is cloned

cd ~/brave-browser/ git remote add bsclifton git@github.com:bsclifton/brave-browser.git git fetch bsclifton

root for the brave-core repo

cd src/brave git remote add bsclifton git@github.com:bsclifton/brave-core.git git fetch bsclifton I think the process in the documentation should be consistent and unified.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

Thanks @bridiver, in that case I think this section of the documentation should be updated:

git clone git@github.com:brave/brave-browser.git
cd brave-browser
npm install

# this takes 30-45 minutes to run
# the Chromium source is downloaded which has a large history
npm run init

It does not even match with this section:

# root where project is cloned
cd ~/brave-browser/
git remote add bsclifton git@github.com:bsclifton/brave-browser.git
git fetch bsclifton
# root for the `brave-core` repo
cd src/brave
git remote add bsclifton git@github.com:bsclifton/brave-core.git
git fetch bsclifton

I think the process in the documentation should be consistent and unified.

this takes 30-45 minutes to run

Anyway, if this can be avoided it’s beneficial for everyone.