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

chore(NA): manage npm dependencies within bazel #92864

Merged
merged 44 commits into from
Mar 3, 2021

Conversation

mistic
Copy link
Member

@mistic mistic commented Feb 25, 2021

One step forward on #69706

That PR introduces a complete Bazel WORKSPACE and by declaring an yarn_install repo rule within a managed_directory it makes Bazel the one responsible for calling the yarn install where previously it was @kbn/pm responsibility.

The yarn_install rule is declaring a data dependency on node_modules/.yarn-integrity so we can still have the yarn install running after an yarn kbn clean otherwise bazel won't install the dependencies as for its state nothing changed within a yarn kbn clean (package.json and yarn.lock file don't present any changes).

By having Bazel managing our dependencies installation we will have the ability to declare fine grained dependencies within our Bazel rules as BUILD files will be generated in the external @npm Bazel repository as part of the workspace installation.

It will allow us to start introduce packages to being able to built within Bazel as soon as our RFC lands.

@mistic mistic added chore Team:Operations Team label for Operations Team v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.13.0 labels Feb 25, 2021
@tylersmalley
Copy link
Contributor

I believe in the preinstall_check we will want to prevent using yarn install directly, right?

diff --git a/preinstall_check.js b/preinstall_check.js
index d6d0cdf1ffb..41049b72151 100644
--- a/preinstall_check.js
+++ b/preinstall_check.js
@@ -33,9 +33,10 @@
       return;
     }
 
-    if (argv.cooked.includes('install')) {
-      console.log('\nWARNING: When installing dependencies, prefer `yarn kbn bootstrap`\n');
-    }
+if (argv.cooked.includes('install')) {
+  console.error('\nWhen installing dependencies, use `yarn kbn bootstrap`\n');
+  process.exit(1);
+}
   } catch (e) {
     // if it fails we do nothing, as this is just intended to be a helpful message
   }

.bazelrc Show resolved Hide resolved
@tylersmalley
Copy link
Contributor

@jbudz, do you have a windows VM to test this on?

@jbudz
Copy link
Member

jbudz commented Mar 3, 2021

I do - testing now.

edit: Windows setup is working

@mistic
Copy link
Member Author

mistic commented Mar 3, 2021

@tylersmalley regarding the preinstall check, I believe even if you use yarn install everything will be okay as you only need the Bazel generated build files for each node_module for Bazel compilations (and when they are in fault Bazel will re run an install to generate them). In any case, do you still prefer to log the error in the preinstall check?

@tylersmalley
Copy link
Contributor

@mistic, then no need to prevent it if it doesn't create issues.

.yarnrc Outdated Show resolved Hide resolved
@mistic
Copy link
Member Author

mistic commented Mar 3, 2021

@tylersmalley just tested it and I can confirm it doesnt cause issues

@mistic mistic added the auto-backport Deprecated - use backport:version if exact versions are needed label Mar 3, 2021
@mistic mistic enabled auto-merge (squash) March 3, 2021 15:42
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@mistic mistic merged commit 86f1684 into elastic:master Mar 3, 2021
@mistic mistic added auto-backport Deprecated - use backport:version if exact versions are needed and removed auto-backport Deprecated - use backport:version if exact versions are needed labels Mar 3, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Mar 3, 2021
* chore(NA): full WORKSPACE.bazel logic plus manage yarn dependencies with Bazel

* chore(NA): update BUILD.bazel files comments on root and packages

* chore(NA): add workspace file with useful data

* chore(NA): install deps through bazel

* chore(NA): update workspace file

* chore(NA): update into last rules nodejs

* chore(NA): ensure bazel always run yarn install

* chore(NA): support offline mode

* chore(NA): remove elastic-datemath

* chore(NA): restore bazel 4.0.0

* chore(NA): update kbn pm dist

* chore(NA): introduce force-install command

* docs(NA): update docs with new yarn kbn bootstrap flags

* chore(NA): use path.resolve on kbn bootstrap integrity check verification

* chore(NA): update .yarnrc

Co-authored-by: Tyler Smalley <[email protected]>

* chore(NA): change cli argument typo

* chore(NA): fix spacing on kbn pm cli

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Tyler Smalley <[email protected]>
@kibanamachine
Copy link
Contributor

💚 Backport successful

7.x / #93474

Successful backport PRs will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Mar 3, 2021
* chore(NA): full WORKSPACE.bazel logic plus manage yarn dependencies with Bazel

* chore(NA): update BUILD.bazel files comments on root and packages

* chore(NA): add workspace file with useful data

* chore(NA): install deps through bazel

* chore(NA): update workspace file

* chore(NA): update into last rules nodejs

* chore(NA): ensure bazel always run yarn install

* chore(NA): support offline mode

* chore(NA): remove elastic-datemath

* chore(NA): restore bazel 4.0.0

* chore(NA): update kbn pm dist

* chore(NA): introduce force-install command

* docs(NA): update docs with new yarn kbn bootstrap flags

* chore(NA): use path.resolve on kbn bootstrap integrity check verification

* chore(NA): update .yarnrc

Co-authored-by: Tyler Smalley <[email protected]>

* chore(NA): change cli argument typo

* chore(NA): fix spacing on kbn pm cli

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Tyler Smalley <[email protected]>

Co-authored-by: Tiago Costa <[email protected]>
Co-authored-by: Tyler Smalley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed chore release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team v7.13.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants