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

process: sync commits from Aspect-internal silo #749

Merged
merged 35 commits into from
Sep 28, 2024

Conversation

@jbedard jbedard marked this pull request as draft September 27, 2024 22:14
Copy link

aspect-workflows bot commented Sep 27, 2024

Test

132 test targets passed

Targets
//gazelle/bzl:defaultvisibility_test [k8-fastbuild]                     146ms
//gazelle/bzl:empty_test [k8-fastbuild]                                 215ms
//gazelle/bzl:import_test [k8-fastbuild]                                389ms
//gazelle/bzl:multidir_test [k8-fastbuild]                              125ms
//gazelle/bzl:nobuildfiles_test [k8-fastbuild]                          112ms
//gazelle/bzl:private_test [k8-fastbuild]                               130ms
//gazelle/bzl:tests_test [k8-fastbuild]                                 155ms
//gazelle/bzl:workspace_name_test [k8-fastbuild]                        135ms
//gazelle/common/git:git_test [k8-fastbuild]                            260ms
//gazelle/common/starlark/utils:utils_test [k8-fastbuild]               279ms
//gazelle/common/starlark:starlark_test [k8-fastbuild]                  196ms
//gazelle/js/typescript:typescript_test [k8-fastbuild]                  131ms
//gazelle/js:declare_module_types_test [k8-fastbuild]                   258ms
//gazelle/js:dynamic_import_test [k8-fastbuild]                         210ms
//gazelle/js:gazelle_exclude_directive_test [k8-fastbuild]              219ms
//gazelle/js:gazelle_generation_mode_legacy_test [k8-fastbuild]         234ms
//gazelle/js:gazelle_ignore_directive_test [k8-fastbuild]               204ms
//gazelle/js:gazelle_keep_test [k8-fastbuild]                           265ms
//gazelle/js:groups_configs_test [k8-fastbuild]                         207ms
//gazelle/js:groups_deps_test [k8-fastbuild]                            219ms
//gazelle/js:ignore_config_files_test [k8-fastbuild]                    641ms
//gazelle/js:ignore_import_directive_test [k8-fastbuild]                238ms
//gazelle/js:js_test [k8-fastbuild]                                     155ms
//gazelle/js:node_native_test [k8-fastbuild]                            240ms
//gazelle/js:npm_link_all_packages_test [k8-fastbuild]                  273ms
//gazelle/js:npm_package_target_enabled_test [k8-fastbuild]             215ms
//gazelle/js:npm_package_target_name_test [k8-fastbuild]                378ms
//gazelle/js:npm_package_target_referenced_test [k8-fastbuild]          210ms
//gazelle/js:npm_simple_deps_cjs_test [k8-fastbuild]                    232ms
//gazelle/js:npm_types_package_test [k8-fastbuild]                      249ms
//gazelle/js:parse_errors_test [k8-fastbuild]                           257ms
//gazelle/js:pnpm_project_refs_lock5_test [k8-fastbuild]                613ms
//gazelle/js:pnpm_project_refs_lock6_test [k8-fastbuild]                224ms
//gazelle/js:pnpm_project_refs_lock9_test [k8-fastbuild]                312ms
//gazelle/js:pnpm_workspace_rerooted_test [k8-fastbuild]                212ms
//gazelle/js:resolve_directive_test [k8-fastbuild]                      207ms
//gazelle/js:resolve_order_test [k8-fastbuild]                          252ms
//gazelle/js:rules_conflicting_name_mapped_kind_test [k8-fastbuild]     153ms
//gazelle/js:rules_conflicting_name_nojs_test [k8-fastbuild]            147ms
//gazelle/js:rules_conflicting_name_test [k8-fastbuild]                 120ms
//gazelle/js:rules_ordering_test [k8-fastbuild]                         232ms
//gazelle/js:simple_dts_only_dep_test [k8-fastbuild]                    301ms
//gazelle/js:simple_extra_files_test [k8-fastbuild]                     267ms
//gazelle/js:simple_file_exts_test [k8-fastbuild]                       414ms
//gazelle/js:simple_globs_keep_test [k8-fastbuild]                      409ms
//gazelle/js:simple_globs_test [k8-fastbuild]                           237ms
//gazelle/js:simple_import_generated_test [k8-fastbuild]                382ms
//gazelle/js:simple_imports_cjs_test [k8-fastbuild]                     379ms
//gazelle/js:simple_imports_dynamic_test [k8-fastbuild]                 227ms
//gazelle/js:simple_imports_test [k8-fastbuild]                         315ms
//gazelle/js:simple_json_import_test [k8-fastbuild]                     622ms
//gazelle/js:simple_new_file_test [k8-fastbuild]                        212ms
//gazelle/js:simple_rule_naming_directives_test [k8-fastbuild]          2s
//gazelle/js:tests_initial_test [k8-fastbuild]                          476ms
//gazelle/js:tests_new_test [k8-fastbuild]                              358ms
//gazelle/js:tests_nolib_test [k8-fastbuild]                            208ms
//gazelle/js:tests_simple_test [k8-fastbuild]                           320ms
//gazelle/js:tests_subdir_test [k8-fastbuild]                           229ms
//gazelle/js:tests_subproject_test [k8-fastbuild]                       302ms
//gazelle/js:ts_proto_library_test [k8-fastbuild]                       351ms
//gazelle/js:tsconfig_attrs_inherited_test [k8-fastbuild]               320ms
//gazelle/js:tsconfig_baseurl_test [k8-fastbuild]                       1s
//gazelle/js:tsconfig_invalid_test [k8-fastbuild]                       205ms
//gazelle/js:tsconfig_jsx_test [k8-fastbuild]                           336ms
//gazelle/js:tsconfig_lax_json_test [k8-fastbuild]                      275ms
//gazelle/js:tsconfig_outdir_test [k8-fastbuild]                        207ms
//gazelle/js:tsconfig_pnpm_ref_rerooted_test [k8-fastbuild]             166ms
//gazelle/js:tsconfig_pnpm_ref_test [k8-fastbuild]                      140ms
//gazelle/js:tsconfig_rootdir_test [k8-fastbuild]                       197ms
//gazelle/js:tsconfig_rootdirs_test [k8-fastbuild]                      209ms
//gazelle/js:tsconfig_tslib_test [k8-fastbuild]                         879ms
//gazelle/js:validate_import_statements_off_test [k8-fastbuild]         760ms
//gazelle/js:validate_import_statements_test [k8-fastbuild]             394ms
//gazelle/js:validate_import_statements_warn_test [k8-fastbuild]        2s
//gazelle/js:visibility_test [k8-fastbuild]                             617ms
//gazelle/kotlin:bin_test [k8-fastbuild]                                139ms
//gazelle/kotlin:gazelle_disable_conflict_test [k8-fastbuild]           2s
//gazelle/kotlin:gazelle_disable_test [k8-fastbuild]                    125ms
//gazelle/kotlin:gazelle_exclude_directive_test [k8-fastbuild]          145ms
//gazelle/kotlin:ignore_config_files_test [k8-fastbuild]                660ms
//gazelle/kotlin:kotlin_test [k8-fastbuild]                             191ms
//gazelle/kotlin:local_deps_test [k8-fastbuild]                         2s
//gazelle/kotlin:native_deps_test [k8-fastbuild]                        132ms
//gazelle/kotlin:rules_conflicting_name_mapped_kind_test [k8-fastbuild] 749ms
//gazelle/kotlin:rules_conflicting_name_test [k8-fastbuild]             382ms
//gazelle/kotlin:simple_file_test [k8-fastbuild]                        130ms
//integration_tests/aspect:configure_test [k8-fastbuild]                31s
//integration_tests/aspect:flags_test [k8-fastbuild]                    2m 20s
//integration_tests/aspect:info_test [k8-fastbuild]                     41s
//integration_tests/aspect:lint_test [k8-fastbuild]                     1m 5s
//integration_tests/aspect:reenter_test [k8-fastbuild]                  44s
//integration_tests/aspect:version_test [k8-fastbuild]                  35s
//pkg/aspect/build:build_test [k8-fastbuild]                            206ms
//pkg/aspect/clean:clean_test [k8-fastbuild]                            132ms
//pkg/aspect/cquery:cquery_test [k8-fastbuild]                          130ms
//pkg/aspect/lint:lint_test [k8-fastbuild]                              177ms
//pkg/aspect/outputs:outputs_test [k8-fastbuild]                        318ms
//pkg/aspect/run:run_test [k8-fastbuild]                                220ms
//pkg/aspect/version:version_test [k8-fastbuild]                        241ms
//pkg/bazel:bazel_test [k8-fastbuild]                                   954ms
+ 32 other targets

Total test execution time was 7m 20s. 91 tests (40.8%) were fully cached saving 28s.


Buildifier      Format

@jbedard jbedard force-pushed the 788EA150BB0FAAB4A3FF81F26DCBE189 branch from 3163857 to 0328923 Compare September 28, 2024 05:12
jbedard and others added 27 commits September 27, 2024 22:15
Currently in silo configure spams stdout complaining about a tsconfig
`extends` not being found. This reproduces something similar, although
not the exact same, in the tsconfig unit tests and removes the stdout
spam when run on silo.

---

### Changes are visible to end-users: no

### Test plan

- New test cases added
- Manual testing; please provide instructions so we can reproduce:
`bazel run //:gazelle` to verify stdout spam is gone

GitOrigin-RevId: d909e21d4b68d48920f01eff25731eae363d1afd
…486)

A long long time ago we had both an esbuild and tree-sitter parser,
eventually moving to tree-sitter and dropping the esbuild one. With only
a single parser there is no need for the interface and extra logic to
separate tests from that one implementation etc.

---

### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases

GitOrigin-RevId: ad5cfda8f0accc61bcb6072a7cd59e3f70f80b61
This was a request from anduril which seemed simple enough, and provides
a path to potentially remove the "referenced" option in the future.

---

### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases
- New test cases added

GitOrigin-RevId: 4370b96c41e0cdbbdc338d5c433c1ee0128a7985
Fixes aspect-build/silo#6545.

---

### Changes are visible to end-users: no

(lint not yet released to customers)

### Test plan

- Covered by existing test cases
- Manual testing; please provide instructions so we can reproduce:

cut a release and test in bazel-lib where we have a repro for the issue.
observe that it no longer fails in this terrible way:
https://app.circleci.com/pipelines/github/aspect-build/bazel-lib/3762/workflows/9c17048e-05c7-4572-9e97-b5b3edbdd2da/jobs/23060

GitOrigin-RevId: 8eed449845d62e98468a3e445b65768c2fabd350
We were not passing through `--` when calling the bazel build from the
CLI lint task

Closes aspect-build/silo#6543

---

### Changes are visible to end-users: yes

- Searched for relevant documentation and updated as needed: no
- Breaking change (forces users to change their own code or config): no
- Suggested release notes appear below: yes

Fixes a bug with `aspect lint` where the `--` argument was not being
used correctly

### Test plan

- Manual testing; please provide instructions so we can reproduce:

This has not been tested beyond some basic local tests

GitOrigin-RevId: ef971058163b5f097e4f6f541c32405da8815100
Passthru all aspect-cli args to gazelle to support updating only
specific directories.

---

### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases
- Manual testing; please provide instructions so we can reproduce: run
`//cli/pro cli` and ensure it only updates that directory and has a
noticeable performance improvement

GitOrigin-RevId: f1e79d2a2ac888488f463d29e014617221fd70b5
…577)

This restores previous behaviour.

---

### Changes are visible to end-users: no

### Test plan

- Manual testing; please provide instructions so we can reproduce: run
`bazel configure` on a repo extending non-existing config while logging
is enabled >=Warn

GitOrigin-RevId: 732fbf17546997c925cd2fa23d4b0f97fca0e5b5
…(#6578)

The last attempt at the fix failed. Still as the "interrupted" failure
on 5.11.0-rc0:
https://app.circleci.com/pipelines/github/aspect-build/bazel-lib/3779/workflows/3e23685c-5b9f-4dbf-8f55-bce4af9d0aca/jobs/23251

This should do the trick.

---

### Changes are visible to end-users: yes

- Searched for relevant documentation and updated as needed: yes
- Breaking change (forces users to change their own code or config): no
- Suggested release notes appear below: yes

Fix bug where processing lint results was interrupted.

### Test plan

- Covered by existing test cases
- New test cases added
- Manual testing; please provide instructions so we can reproduce:

Cut a dev release and ship to bazel-lib where we have the repro

GitOrigin-RevId: 4ca32d1230daddb944de6ff5b42ee70962b061bb
A few fixes for lint 5.11

1. don't interrupt on `<-runner.ctx.Done()`. this context comes from
cobra `cmd.Context()` under the hood and fires right away. not sure why
but simpler to just not listen for that signal since the timer is a
short 100ms timer than try dig into why. the CLI should _not_ exit
before it tries to read all the lint results files so nothing should
kill the context
2. don't quote the `remote_download_regex`. the quotes are breaking it.
not sure why on this one either but it was easy to determine
experimentally in bazel-lib where we have a consistent repro of the
results files _not_ being on disk.

---

### Changes are visible to end-users: yes

- Searched for relevant documentation and updated as needed: yes
- Breaking change (forces users to change their own code or config): no
- Suggested release notes appear below: yes

Fix bug in remote_download_regex in CLIU lint command

### Test plan

- Covered by existing test cases
- Manual testing; please provide instructions so we can reproduce:

Cut dev release, deploy to bazel-lib and check that lint works there.
That is the only place so far that we have a repro for this issue.

GitOrigin-RevId: 11897061291182c7b46f57b28b0011a75a15d7ce
To include the latest changes including many performance improvements.

---

- Covered by existing test cases

GitOrigin-RevId: 662266cad761461915f71cb79b08b3a326cfa338
Support imports with the `.{cjs,mjs}` statements.

---

### Changes are visible to end-users: no

### Test plan

- New test cases added

GitOrigin-RevId: 86c17a732b1c7c995a88b3b9a677803ce03ec046
…eprecrated (#6600)

`rules_lint_report` is a deprecated and a copy of `rules_lint_machine`
which we should use instead.

https://github.com/aspect-build/rules_lint/blob/b9068fc116822c456253ee3d4f8fface7291e095/lint/private/lint_aspect.bzl#L120

---

### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases

GitOrigin-RevId: 32b2942c39d69a2b7e9a789d7d72b170267d7a08
Release notes: https://go.dev/blog/go1.23

Also upgrade hay-kot/scaffold so I can use some validator features in
the 'aspect init' template.

---

- Covered by existing test cases
- Manual testing:

built CLI, ran `init` against current HEAD template, prompts look good
and it creates working app

GitOrigin-RevId: 3f0eafc08109d98b07bdc1791a6dd4bf18c55a9e
We were always showing the linting sarif output from the CLI. Instead
default to the human readable version and request the machine readable
one from workflows

---

- Searched for relevant documentation and updated as needed: no
- Breaking change (forces users to change their own code or config): yes
- Suggested release notes appear below: yes

The CLI now outputs linting results in human readable form by default.
To get the machine readable version, use the `--machine` flag

- Manual testing; please provide instructions so we can reproduce:

Running against aspect-build/bazel-examples#346
with and without `--machine`

GitOrigin-RevId: d5c11728f628d459d59d59251b84fe839fd2e5e4
This case will be hit more often then I thought such as when extra
sources are added to `ts_project(srcs)` with things like `# keep`.

---

### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases

GitOrigin-RevId: 1d6a88ef9b3b27fbb4338ae82e9e7d9067abae11
Run actions across plugins in parallel.
Run queries across source files in parallel.

---

### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases

GitOrigin-RevId: 6aacc9de4782bdabe6f2c86139b79187b70f5f0b
…subdir (#6574)

A copy of another test but with the lockfile in a subdir, and more
tests, and minor naming/comments.

---

### Changes are visible to end-users: no

### Test plan

- New test cases added

GitOrigin-RevId: 411333a95bcb148edb0137bee5b84f2c4a2cef3c
This removes support for using our gazelle plugins without our gazelle
patches. If someone wants to compile our gazelle plugins into a
`gazelle_binary` for use outside the CLI they will have to patch gazelle
like we do within the CLI for features such as `#
gazelle:generation_mode`.

I think some of our patches might get more complicated so I don't want
our gazelle plugins to have to support both patched and non-patched.
Most of those patches will most likely have PRs on the gazelle repo but
I don't trust that those will merge fast enough.

---

- Searched for relevant documentation and updated as needed: yes
- Breaking change (forces users to change their own code or config): yes
- Suggested release notes appear below: yes

Aspect gazelle languages manually compiled into a `gazelle_binary` will
no longer have full functionality without applying the aspect-cli
gazelle patches.

- Covered by existing test cases

GitOrigin-RevId: 4733a6fa176f5395d29cc0ac82f7de05621b9bf2
Normalize labels in starzelle before they are put into rule attributes.

This refactors some common gazelle code, mainly to reduce+reuse a bit
more, but only the starzelle gazelle plugin has side-effects atm to
normalize labels better when manually put into attributes.

---

### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases

GitOrigin-RevId: 1491b9bf509e39b67315207f57c625deb6e87f86
…ared across plugins (#6723)

This is a generic directive used across gazelle languages which I hope
to upstream into gazelle soon. For now this merges the 2 versions we
have in silo.

Having multiple languages with alternate versions of this causes
conflicts. One language should not be able to declare that a BUILD
should be created while another one says it should not be created.

---

### Changes are visible to end-users: yes

- Searched for relevant documentation and updated as needed: yes
- Breaking change (forces users to change their own code or config): yes
- Suggested release notes appear below: yes

The gazelle `js_generation_mode none|directory` has been replaced with
`generation_mode create|update`

### Test plan

- Covered by existing test cases
- New test cases added

GitOrigin-RevId: b912ddcc8ac4d58d20467b7d9a4e6ab38f55f93c
### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases

GitOrigin-RevId: de7688f7b2ef158c5995fb8c5112eda1f94b98db
Code changes are all copied from gazelle to vendored code.

---

### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases

GitOrigin-RevId: 6f9cdacb76eded5d3f442ed310b6d9f4e1cd7de6
GitOrigin-RevId: f1d63cd3af4b770e2f0eb6867c90af50efe73c09
jbedard and others added 8 commits September 27, 2024 22:15
Close #745

Automatically generate npm package deps (`js_library(deps)` or
`npm_package(srcs)`) based on package.json references to
source/generated/compiled files.

---

### Changes are visible to end-users: no

### Test plan

- New test cases added

GitOrigin-RevId: fbb793917693d6e241c52b71ae882650d3bf7f84
---

- Covered by existing test cases

GitOrigin-RevId: 231e203af2f42a399884c29ed7488bb6c4018198
At some point the gazelle method of updated the "known good" BUILDs
started failing which seems to be related to how we were copying test
data to bin. This removes the copying and uses `generule` to do the file
renaming where required.

---

### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases

GitOrigin-RevId: c123c08f8dc1cf963e22b4392123a85cea7de1b2
### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases
- New test cases added

GitOrigin-RevId: d669a7ad782c1217956c6bf1306c96bf9904ca9c
The package.json [exports
field](https://nodejs.org/docs/latest-v22.x/api/packages.html#exports)
has a few formats we must support.

---

### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases
- New test cases added

GitOrigin-RevId: ac473110a61016e13ddab401d6094cc49f78281e
… gazelle feature implementation (#6873)

This aligns the implementation more with what I'm trying to merge into
gazelle: bazelbuild/bazel-gazelle#1908

There are no user facing changes here but the implementation is simpler
and should improve performance for many reasons:
* the .gitignore files are only `os.Open`ed once per directory (not per
directory * per language)
* the .gitignore files are only parsed once instead of once per gazelle
language
* the .gitignore patterns are only executed once because it is done
within the (patched) gazelle walk
* .gitignore-d directories are not recursed into at all because they are
filtered in the gazelle fs walk
* ...

---

### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases

GitOrigin-RevId: 27fe8377ca89de91d3c95105ab37adea9347e6ab
### Changes are visible to end-users: yes

- Searched for relevant documentation and updated as needed: yes
- Breaking change (forces users to change their own code or config): no
- Suggested release notes appear below: yes

Disable python extension on windows

### Test plan

- Covered by existing test cases

GitOrigin-RevId: 631b85f43769ea877ebd14d6b4df7be48077dd64
The package.json deps should support the same `js_ignore_imports`
directive as all other js imports.

Ideally this will move to a more common location but this is the quick
fix.

---

### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases
- New test cases added

GitOrigin-RevId: cfa2a62823454674453f3d6458e52ed30fbe0581
@jbedard jbedard force-pushed the 788EA150BB0FAAB4A3FF81F26DCBE189 branch from 0328923 to bcd669b Compare September 28, 2024 05:15
@jbedard jbedard marked this pull request as ready for review September 28, 2024 05:17
@jbedard jbedard enabled auto-merge (rebase) September 28, 2024 05:54
@jbedard jbedard merged commit 6af1457 into main Sep 28, 2024
9 checks passed
@jbedard jbedard deleted the 788EA150BB0FAAB4A3FF81F26DCBE189 branch September 28, 2024 16:26
return nil, false
}

+ // PATCH(gitignore) ---
Copy link

@gonzojive gonzojive Sep 29, 2024

Choose a reason for hiding this comment

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

FYI, I left a comment in bazelbuild suggesting gazelle expose an API for configuring ignore: bazelbuild/bazel-gazelle#1166 (comment) - ideally there would be no need to patch.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @gonzojive, note I've also implemented it within gazelle and I'm trying to get it reviewed+merged: bazelbuild/bazel-gazelle#1908

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants