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

Remove bazel usage from the repo #1580

Closed
howardjohn opened this issue Aug 2, 2019 · 9 comments · Fixed by #4116
Closed

Remove bazel usage from the repo #1580

howardjohn opened this issue Aug 2, 2019 · 9 comments · Fixed by #4116
Labels
lifecycle/staleproof Indicates a PR or issue has been deemed to be immune from becoming stale and/or automatically closed

Comments

@howardjohn
Copy link
Member

No other repo in istio uses bazel. As far as I know there is no reason we need bazel here, and it makes the repo unfamiliar for Istio contributors.

If possible we should remove bazel.

I am not familiar enough with bazel to know how feasible this is and if its as simple as rm all_bazel_stuff and replace bazel test with go test, etc

@clarketm
Copy link
Member

clarketm commented Aug 7, 2019

@howardjohn - I can help with this. Depending on the particular bazel rule we can translate to the equivalent, underlying command (e.g. go_test(...) -> go test ...). Is the desire to replace bazel with make for everything: building, testing, running or just testing?

@clarketm
Copy link
Member

clarketm commented Aug 23, 2019

@sebastienvas @fejta

Since #1631, it would appear bazel no longer works yet there are still artifacts (i.e. subprojects withBUILD.bazel files like boskos) in the repo that depend on it.

It would be nice to come to a compromise of if or to what extent we want to use bazel in the istio/test-infra repo. At the very least, we should keep it in a healthy state for the components that depend on it.

@howardjohn
Copy link
Member Author

That is exactly why I created #1635, to remove the BUILD.bazel files as they are actively misleading. This change was blocked.

@duderino
Copy link

No other repo in istio uses bazel.

istio/proxy does, but it's still OK to remove bazel from istio/test-infra. But let's definitely not remove bazel from istio/proxy ;)

@sebastienvas
Copy link
Contributor

Bazel was used for 2 + years and nobody complained.

I find it really sad. the time gained when working with docker images, and daily workflows for me is priceless. Anyways I am doing one or two contribution a month to fix stuff, so I may not be the standard user. I think @fejta has made some bazel contrib in the last months so He may have more context.

@fejta
Copy link
Contributor

fejta commented Aug 23, 2019

I am opposed to removing bazel usage from this repo

@istio-policy-bot istio-policy-bot added the lifecycle/needs-triage Indicates a new PR or issue needs to be triaged label Oct 30, 2019
@clarketm clarketm removed the lifecycle/needs-triage Indicates a new PR or issue needs to be triaged label Mar 26, 2020
@istio-policy-bot istio-policy-bot added the lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. label Mar 26, 2020
@clarketm clarketm added lifecycle/staleproof Indicates a PR or issue has been deemed to be immune from becoming stale and/or automatically closed and removed lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. labels Mar 26, 2020
@clarketm clarketm reopened this Mar 26, 2020
@howardjohn
Copy link
Member Author

Given kubernetes/kubernetes#99561 this seems to be where upstream is moving? Unless k/test-infra will still use bazel

@BenTheElder
Copy link

That KEP expressly covers only github.com/kubernetes/kubernetes. Sub-projects will continue to use their preferred tools. I don't think there's currently a movement to change any other repos.

@howardjohn
Copy link
Member Author

Ack, thanks Ben.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/staleproof Indicates a PR or issue has been deemed to be immune from becoming stale and/or automatically closed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants