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

Fix paths for kibana packages used by plugins #57097

Merged
merged 5 commits into from
Feb 12, 2020

Conversation

shaileshcheke
Copy link
Contributor

Summary

When we try to install kibana plugin, This plugin is using link: dependencies for non-Kibana packages is thrown by prepare_project_dependencies.ts.
The reason is kibana plugins uses link dependencies in package.json for kibana packages ex: "@kbn/i18n": "link:../../packages/kbn-i18n".
When we try to install such plugins, function prepareExternalProjectDependencies from prepare_project_dependencies.ts(@kbn-pm) is executed which check for dependencies that are listed in package.json.
and it throws error mentioned above if we use link dependencies for non kibana package. It checks if dependency is kibana package in function isKibanaDep which checks if ../../kibana is there in dependency version string.
When issue #33728 was resolved, paths in package_template.json in plugin generator("@kbn/i18n": "link:../../kibana/packages/kbn-i18n") was replaced by @kbn/i18n": "link:../../packages/kbn-i18n".
It seems from #33728 and respective pull request that kibana packages path is ../../packages(since v7.2.0). Hence path in prepare_project_dependencies.ts needs to be fixed. #

@shaileshcheke shaileshcheke requested a review from a team as a code owner February 7, 2020 13:27
@elasticmachine
Copy link
Contributor

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@mistic mistic added the Team:Operations Team label for Operations Team label Feb 7, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

@mistic mistic changed the title Fix paths for kibana packages in prepare_project_dependencies (#40858). Fix paths for kibana packages used by plugins Feb 7, 2020
@mistic

This comment has been minimized.

@mistic

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@mistic

This comment has been minimized.

@mistic

This comment has been minimized.

@mistic
Copy link
Member

mistic commented Feb 7, 2020

@shaileshcheke could you please run yarn kbn run build -i @kbn/pm and also commit the changes resulting from that operation please?

@shaileshcheke
Copy link
Contributor Author

@mistic When I executed yarn kbn run build -i @kbn/pm first time, build was successful. I executed it again and getting Error: Cannot find module 'static-extend'. When I googled it, I came across https://discuss.elastic.co/t/error-while-building-package-from-source-code/188432/3. @LeeDr suggested to git reset --hard HEAD and then execute build on windows. I followed steps and committed results. But still CI/Build is failing. I am using Windows 10. Is it because of windows 10?

@mistic
Copy link
Member

mistic commented Feb 10, 2020

@shaileshcheke try to run git reset --hard HEAD, then yarn kbn bootstrap and finally yarn kbn run build -i @kbn/pm. Let me know the results

@shaileshcheke
Copy link
Contributor Author

@mistic I executed git reset --hard HEAD followed by yarn kbn bootstrap. After that I executed yarn kbn run build -i @kbn/pm and it worked successfully. But when I executed yarn kbn run build -i @kbn/pm again, it fails with Error: Cannot find module 'static-extend'.

@mistic

This comment has been minimized.

@mistic

This comment has been minimized.

@mistic
Copy link
Member

mistic commented Feb 10, 2020

@shaileshcheke let's try to do one last thing please, if that fails, I'll try to reproduce the PR.
Can we do the following please?

1 - git reset --hard 889a5cc && git push --force
2 - yarn kbn bootstrap
3 - cd packages/kbn-pm && yarn build
4 - git add -A && git commit...

@shaileshcheke
Copy link
Contributor Author

@mistic Please check.

@mistic

This comment has been minimized.

@mistic
Copy link
Member

mistic commented Feb 10, 2020

@shaileshcheke you are doing everything well. Probably there is some probably when generating that file from windows that we need to look at. Meanwhile, could you please cherry pick the commit 985c85c I have in https://github.com/mistic/kibana/tree/recreation-of-57097 and push it to that PR? That would make the build to pass and allow us to move on with that PR.

Thank you very much for your help!

@shaileshcheke
Copy link
Contributor Author

@mistic Committed changes after cherry-pick. Please check.

Thanks

@mistic
Copy link
Member

mistic commented Feb 11, 2020

@elasticmachine merge upstream

@mistic
Copy link
Member

mistic commented Feb 11, 2020

Jenkins test this

Copy link
Member

@mistic mistic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@tylersmalley
Copy link
Contributor

@joshdover I want to check with you before moving forward with this, since I know we have gone back and forth with where 3rd party plugins should live while in development. This PR looks to solve plugins being generated with scripts/generate_plugin which live in plugins/*, but I believe it would then break those still residing in kibana-extra/*.

@tylersmalley tylersmalley added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Feb 12, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@mistic mistic merged commit 3c3d0b5 into elastic:master Feb 12, 2020
mistic added a commit to mistic/kibana that referenced this pull request Feb 12, 2020
* Fix paths for kibana packages in prepare_project_dependencies(elastic#40858).

* Dist/index.js after yarn build.

* chore(NA): correctly include kbm pm new dist file

* Commit after cherry-pick 985c85c from recreation branch.

Co-authored-by: Tiago Costa <tiagoffcc@hotmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@mistic
Copy link
Member

mistic commented Feb 12, 2020

7.x: ebb49b7
7.6: e1fdb21

mistic added a commit to mistic/kibana that referenced this pull request Feb 12, 2020
* Fix paths for kibana packages in prepare_project_dependencies(elastic#40858).

* Dist/index.js after yarn build.

* chore(NA): correctly include kbm pm new dist file

* Commit after cherry-pick 985c85c from recreation branch.

Co-authored-by: Tiago Costa <tiagoffcc@hotmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
mistic added a commit that referenced this pull request Feb 12, 2020
* Fix paths for kibana packages in prepare_project_dependencies(#40858).

* Dist/index.js after yarn build.

* chore(NA): correctly include kbm pm new dist file

* Commit after cherry-pick 985c85c from recreation branch.

Co-authored-by: Tiago Costa <tiagoffcc@hotmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Shailesh cheke <shailesh.cheke@gmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
mistic added a commit that referenced this pull request Feb 12, 2020
* Fix paths for kibana packages in prepare_project_dependencies(#40858).

* Dist/index.js after yarn build.

* chore(NA): correctly include kbm pm new dist file

* Commit after cherry-pick 985c85c from recreation branch.

Co-authored-by: Tiago Costa <tiagoffcc@hotmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Shailesh cheke <shailesh.cheke@gmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@joshdover
Copy link
Contributor

@joshdover I want to check with you before moving forward with this, since I know we have gone back and forth with where 3rd party plugins should live while in development. This PR looks to solve plugins being generated with scripts/generate_plugin which live in plugins/*, but I believe it would then break those still residing in kibana-extra/*.

Kibana Platform plugins are only supported in plugins/*, but maybe we should change this to work for both new and legacy plugins until legacy plugin support is removed (#56205). Maybe this:

const isKibanaDep = (depVersion: string) =>
  // For ../kibana-extra/ directory (legacy only)
  depVersion.includes('../../kibana/packages/') ||
  // For plugins/ directory
  depVersion.includes('../../packages/');

@mistic
Copy link
Member

mistic commented Feb 12, 2020

Thanks for the feedback @joshdover, I'll make that change and add you as a reviewer 😃

@shaileshcheke
Copy link
Contributor Author

@joshdover Thanks for feedback. Thanks @mistic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore release_note:fix Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Operations Team label for Operations Team v7.6.1 v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants