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

Can we pip install jinja2 and markupsafe instead of vendoring them in? #24891

Closed
cclauss opened this issue Dec 7, 2018 · 4 comments
Closed
Labels
build Issues and PRs related to build files or the CI. discuss Issues opened for discussions and feedbacks. wontfix Issues that will not be fixed.

Comments

@cclauss
Copy link
Contributor

cclauss commented Dec 7, 2018

Related to #21897, #24512, and #24882

Is your feature request related to a problem? Please describe.
Vendoring is already rare in the Python world and the process of removing it is accelerating. Our codebase becomes large and inflexible when we vendor in because we take on board the incompatibilities of third party modules. It requires detective work just to tell what versions of these packages we are using. It takes even more effort to upgrade them for security fixes or functionality improvements. This will slow our migration to Python 3. It also makes our linting and testing more complex because so much third party code is embedded inside of our codebase and the excludes are becoming long and tricky to manage.

A simple requirements.txt file with the contents:

Jinja2==2.10
MarkupSafe==1.1.0

and adding the command pip install -r requirements.txt to our build process would allow us to move all this bloat out of this repo and give us more flexibility to move faster and respond to security threats.

In the future we could continue this process adding other dependencies such as inspector_protocol, gyp, etc.

Describe the solution you'd like
Please describe the desired behavior.
Create requirements.txt, add pip command to our build process, remove those dependencies from our repo.

Describe alternatives you've considered
Please describe alternative solutions or features you have considered.
excludes

@bnoordhuis bnoordhuis added discuss Issues opened for discussions and feedbacks. build Issues and PRs related to build files or the CI. labels Dec 7, 2018
@bnoordhuis
Copy link
Member

One of the goals of the build has always been to have as few external dependencies as possible, to decrease friction for builders and maintenance hassle for maintainers. Examples: openssl, zlib, gyp.

We kind of moved away with that for linting but at least that's our "own turf", so to speak; they're JS dependencies installed through (the vendored copy of) npm.

@sam-github
Copy link
Contributor

pip install -r requirements.txt

Funny, a copule days ago I installed a python tool that's README said "use pip to install". Took me 10 minutes to figure out what package I needed to install pip on Ubuntu.... and then more googling because the python packages wouldn't compile because the -dev library packages they needed weren't installed... It worked out in the end, but there are tradeoffs.

Chrome, go, and Node.js are counter-narratives to the "don't vendor" philosophy.

@joyeecheung
Copy link
Member

I think adding one more external dependency that is as complicated as a pip install -r requirements.txt (instead of some simple curling) would make the project harder to contribute. Imagine how it's going to affect a code-and-learn session where I suspect there will be significant amount of people getting confused on how to set up Python and pip properly before they can start building Node.js and actually working on tasks. In that regard, npm install is way less troublesome since our new contributors are likely to have some idea on how to work with npm already.

@bnoordhuis
Copy link
Member

I'll go ahead and say this is unlikely to happen for lack of consensus (or rather: what consensus there is, is in the opposite direction.) Thanks anyway for the suggestion, @cclauss.

@bnoordhuis bnoordhuis added the wontfix Issues that will not be fixed. label Dec 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. discuss Issues opened for discussions and feedbacks. wontfix Issues that will not be fixed.
Projects
None yet
Development

No branches or pull requests

4 participants