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.

Sign up for GitHub

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

Jump to bottom

[internal] go: enable plugin in repo and cleanup test project #13018

Merged
merged 4 commits into from
Sep 27, 2021
Merged

[internal] go: enable plugin in repo and cleanup test project #13018

merged 4 commits into from
Sep 27, 2021

Conversation

tdyas
Copy link
Contributor

@tdyas tdyas commented Sep 27, 2021 •
edited
Loading

The Go plugin is far enough long that we should enable it in the repository. Also cleanup the BUILD files for the test project to match current plugin conventions. The test project's BUILD files are still ignored so that Pants developers do not need to install Go (due to target generation for go_module needing go).

[ci skip-rust]

[ci skip-build-wheels]

enable Go plugin in repo and cleanup test project
afeecd9 
[ci skip-rust]

[ci skip-build-wheels]
@tdyas
Copy link
Contributor Author

tdyas commented Sep 27, 2021

Eager target generation for go_module is requiring that the go tool be available even in the bootstrap shards:


pants.engine.process.BinaryNotFoundError: Cannot find a `go` binary with the expected version of 1.17 (set by `[golang].expected_version`).

Found these `go` binaries, but they had different versions:

  * /bin/go, 1.15
  * /usr/bin/go, 1.15

To fix, please install the expected version (https://golang.org/doc/install) and ensure that it is discoverable via the option `[golang].go_search_paths`, or change `[golang].expected_version`.

https://github.com/pantsbuild/pants/runs/3723406388?check_suite_focus=true#step:14:112

@Eric-Arellano
Copy link
Contributor

That raises a good point. Are we comfortable requiring that Pants contributors have Go 1.17 on their PATH?

@kaos
Copy link
Member

kaos commented Sep 27, 2021

For me that's not an issue. It does pose an interesting question though, as support for more languages are added, it could become a bit of a hassle if every supported language were to be required..

@Eric-Arellano
Copy link
Contributor

Agreed. @kaos the plan is for us to go back to installing Go for you, but that's probably 1+ month out because we need to land some new mechanisms ("immutable caches")

@Eric-Arellano
Copy link
Contributor

I think we should still probably not have the BUILD file activated by default..we can have the backend activated safely, but ignore the BUILD files. Because there will be no Go targets, none of the Go code is used.

That reduces the friction with activating without requiring developers to install Go 1.17.

@tdyas
Copy link
Contributor Author

tdyas commented Sep 27, 2021

I think we should still probably not have the BUILD file activated by default..we can have the backend activated safely, but ignore the BUILD files. Because there will be no Go targets, none of the Go code is used.

That reduces the friction with activating without requiring developers to install Go 1.17.

Should I bring back the pants.toml config to ignore the BUILD files under testprojects/src/go or is there some other way to "deactivate" a BUILD file?

@tdyas tdyas merged commit 18e4a6c into pantsbuild:main Sep 27, 2021
@tdyas tdyas deleted the golang_enable_plugin_in_repo branch September 27, 2021 20:04
@Eric-Arellano
Copy link
Contributor

Precisely @tdyas, activate the backend but use build_ignore so it looks like there are no Go targets. Then there's only one step to activate Go, whereas there were two before.

@Eric-Arellano
Copy link
Contributor

Oh great, looks like you did just that :)

@wisechengyi wisechengyi mentioned this pull request Oct 2, 2021
Merged
stuhood pushed a commit that referenced this pull request Oct 2, 2021
@wisechengyi
2.8.0.dev3 prep (#13081)
a30c98d 
* [internal] Run pyupgrade on src/python/pants/backend/python ([#13073](#13073))
* [internal] Re-enable some skipped JVM tests. ([#13074](#13074))
* [internal] Use `DownloadedExternalModules` when analyzing external Go packages ([#13076](#13076))
* [internal] Use `DownloadedExternalModules` during Go target generation ([#13070](#13070))
* [internal] Replace deprecated use of `[pytest] junit_xml_dir` with `[test] xml_dir. ([#13069](#13069))
* [internal] Add `DownloadedExternalModules` for Go ([#13068](#13068))
* [internal] Always use jars on the user classpath, and generalize transitive classpath building ([#13061](#13061))
* Add failing tests for Go external modules ([#13065](#13065))
* [internal] java: fix version in test ([#13064](#13064))
* [internal] Skip additional inference tests ([#13062](#13062))
* [internal] java: enable cycles for file-level targets generated by `java_sources` ([#13058](#13058))
* [internal] Add a `@logging` decorator for tests. ([#13060](#13060))
* [internal] Improve compatibility of nailgun with append only caches, and use them for Coursier ([#13046](#13046))
* [internal] Stop using `go.sum` when generating `_go_external_package` targets ([#13052](#13052))
* [internal] Rename `go_module` target to `go_mod` ([#13053](#13053))
* [internal] Refactor `go/util_rules/external_module.py` ([#13051](#13051))
* [internal] go: add analyzer and rules for test sources ([#13041](#13041))
* [Internal] Refactoring how we integrate with dockerfile ([#13027](#13027))
* [internal] Simplify `go/package_binary.py` ([#13045](#13045))
* [internal] Refactor `OwningGoMod` ([#13042](#13042))
* [internal] Refactor `go_mod.py` ([#13039](#13039))
* [internal] Record metadata on engine-aware params ([#13040](#13040))
* [internal] Test discovery of `go` binary ([#13038](#13038))
* [internal] Extract directory setup for terraform linters / formatters into a separate rule. ([#13037](#13037))
* [internal] java: register dependency inference rules ([#13035](#13035))
* [internal] Add `strutil.bullet_list()` to DRY formatting ([#13031](#13031))
* Minor cleanups for the autoflake linter / formatter. ([#13032](#13032))
* Ensure XML results recorded for both pytest and junit ([#13025](#13025))
* [internal] go: refactor compilation into separate rule ([#13019](#13019))
* [internal] go: refactor link step into separate rule ([#13022](#13022))
* [internal] go: enable plugin in repo and cleanup test project ([#13018](#13018))
* [internal] go: use colon to separate binary name and version ([#13020](#13020))
* [internal] tweak formatting of help text for sourcefile-validation subsystem. ([#13016](#13016))
* [internal] Use system-installed Go rather than installing via Pants ([#13007](#13007))
* Move the `process-execution-local-cleanup` hint to a more specific location. ([#13013](#13013))
* [internal] Split shell targets into atom vs generator ([#12957](#12957))
* Install Go in CI ([#13011](#13011))
* Refresh maintainers list. ([#13004](#13004))
* [internal] Refactor setup of GOROOT and `import_analysis.py` ([#13000](#13000))
* Infer dependencies on COPY of pex binaries for `docker_image`s. ([#12920](#12920))
* Prepare 2.7.0. ([#12995](#12995))
* [internal] jvm: skip JDK tests unless env var set ([#12994](#12994))
* [internal] jvm: limit caching of JDK setup processes ([#12992](#12992))
* [internal] Async-ify `NailgunPool::connect` and `nailgun::CommandRunner`. ([#12990](#12990))
* [internal] Replace `java_library` with `java_source` and `java_sources`, and add `java_test` ([#12976](#12976))
* Prepare 2.7.0rc5. ([#12987](#12987))
* [internal] terraform: refactor parser script into its own file ([#12985](#12985))
* [internal] jvm/java: ensure JDK downloaded in one process ([#12972](#12972))
* add JDK to GitHub Actions CI ([#12977](#12977))
* [internal] Re-enable the `clippy::used_underscore_binding` check. ([#12983](#12983))
* [internal] Use target generation for `_go_external_package` ([#12929](#12929))
* [internal] Bump CI token expiration threshold. ([#12974](#12974))
* [internal] Re-enable the Java backend. ([#12971](#12971))
* [internal] Implement `@union`s via `Query`s ([#12966](#12966))
* Remove `Enriched*Result` classes in favor of `EngineAwareReturnType.cacheable` ([#12970](#12970))
* [internal] Remove spurious `python_tests` directive ([#12968](#12968))
* [internal] Python coverage report generation uses precomputed addresses. ([#12965](#12965))
* Add PackageRootedDependencyMap for mapping inferred Java dependencies ([#12964](#12964))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stuhood stuhood stuhood approved these changes

@Eric-Arellano Eric-Arellano Eric-Arellano approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tdyas @Eric-Arellano @kaos @stuhood