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 #763

Merged
merged 23 commits into from
Oct 24, 2024

Conversation

Copy link

aspect-workflows bot commented Oct 23, 2024

Test

133 test targets passed

Targets
//docs:check_aspect_lint.md [k8-fastbuild]                              112ms
//gazelle/bzl:bazel_tools_test [k8-fastbuild]                           389ms
//gazelle/bzl:empty_test [k8-fastbuild]                                 114ms
//gazelle/bzl:multidir_test [k8-fastbuild]                              351ms
//gazelle/bzl:nobuildfiles_test [k8-fastbuild]                          353ms
//gazelle/bzl:relative_import_test [k8-fastbuild]                       468ms
//gazelle/bzl:simple_test [k8-fastbuild]                                388ms
//gazelle/bzl:tests_test [k8-fastbuild]                                 414ms
//gazelle/bzl:workspace_name_test [k8-fastbuild]                        270ms
//gazelle/js/parser:parser_test [k8-fastbuild]                          690ms
//gazelle/js:declare_module_test [k8-fastbuild]                         268ms
//gazelle/js:declare_module_types_test [k8-fastbuild]                   343ms
//gazelle/js:dynamic_import_test [k8-fastbuild]                         242ms
//gazelle/js:gazelle_disable_conflict_test [k8-fastbuild]               372ms
//gazelle/js:gazelle_disable_test [k8-fastbuild]                        2s
//gazelle/js:gazelle_exclude_directive_test [k8-fastbuild]              381ms
//gazelle/js:gazelle_generation_mode_legacy_test [k8-fastbuild]         444ms
//gazelle/js:gazelle_keep_test [k8-fastbuild]                           646ms
//gazelle/js:gazelle_map_kind_directive_test [k8-fastbuild]             346ms
//gazelle/js:groups_add_remove_rules_test [k8-fastbuild]                581ms
//gazelle/js:groups_configs_test [k8-fastbuild]                         227ms
//gazelle/js:groups_deps_test [k8-fastbuild]                            323ms
//gazelle/js:groups_simple_files_test [k8-fastbuild]                    188ms
//gazelle/js:ignore_config_files_test [k8-fastbuild]                    1s
//gazelle/js:ignore_import_directive_test [k8-fastbuild]                204ms
//gazelle/js:js_test [k8-fastbuild]                                     92ms
//gazelle/js:node_native_test [k8-fastbuild]                            769ms
//gazelle/js:npm_package_deps_test [k8-fastbuild]                       387ms
//gazelle/js:npm_package_lib_target_name_test [k8-fastbuild]            670ms
//gazelle/js:npm_package_target_referenced_test [k8-fastbuild]          933ms
//gazelle/js:npm_simple_deps_cjs_test [k8-fastbuild]                    183ms
//gazelle/js:npm_types_package_test [k8-fastbuild]                      362ms
//gazelle/js:parse_errors_test [k8-fastbuild]                           259ms
//gazelle/js:pnpm_project_refs_lock5_test [k8-fastbuild]                155ms
//gazelle/js:pnpm_project_refs_lock6_test [k8-fastbuild]                765ms
//gazelle/js:pnpm_project_refs_lock9_test [k8-fastbuild]                424ms
//gazelle/js:pnpm_workspace_rerooted_test [k8-fastbuild]                503ms
//gazelle/js:resolve_directive_test [k8-fastbuild]                      317ms
//gazelle/js:rules_conflicting_name_mapped_kind_test [k8-fastbuild]     139ms
//gazelle/js:rules_conflicting_name_nojs_test [k8-fastbuild]            596ms
//gazelle/js:rules_conflicting_name_test [k8-fastbuild]                 165ms
//gazelle/js:rules_ordering_test [k8-fastbuild]                         972ms
//gazelle/js:simple_dts_only_dep_test [k8-fastbuild]                    459ms
//gazelle/js:simple_dts_only_test [k8-fastbuild]                        1s
//gazelle/js:simple_empty_test [k8-fastbuild]                           342ms
//gazelle/js:simple_extra_files_test [k8-fastbuild]                     420ms
//gazelle/js:simple_file_exts_test [k8-fastbuild]                       494ms
//gazelle/js:simple_file_test [k8-fastbuild]                            301ms
//gazelle/js:simple_globs_keep_test [k8-fastbuild]                      360ms
//gazelle/js:simple_import_disabled_test [k8-fastbuild]                 312ms
//gazelle/js:simple_import_generated_test [k8-fastbuild]                417ms
//gazelle/js:simple_imports_cjs_test [k8-fastbuild]                     400ms
//gazelle/js:simple_imports_test [k8-fastbuild]                         278ms
//gazelle/js:simple_json_import_test [k8-fastbuild]                     312ms
//gazelle/js:simple_rule_naming_directives_test [k8-fastbuild]          314ms
//gazelle/js:tests_initial_test [k8-fastbuild]                          404ms
//gazelle/js:tests_new_test [k8-fastbuild]                              375ms
//gazelle/js:tests_nolib_test [k8-fastbuild]                            284ms
//gazelle/js:tests_subdir_test [k8-fastbuild]                           179ms
//gazelle/js:tests_subproject_test [k8-fastbuild]                       515ms
//gazelle/js:ts_proto_library_test [k8-fastbuild]                       694ms
//gazelle/js:tsconfig_baseurl_test [k8-fastbuild]                       230ms
//gazelle/js:tsconfig_invalid_test [k8-fastbuild]                       389ms
//gazelle/js:tsconfig_jsx_test [k8-fastbuild]                           976ms
//gazelle/js:tsconfig_lax_json_test [k8-fastbuild]                      329ms
//gazelle/js:tsconfig_manual_test [k8-fastbuild]                        328ms
//gazelle/js:tsconfig_nomore_configs_test [k8-fastbuild]                430ms
//gazelle/js:tsconfig_outdir_test [k8-fastbuild]                        830ms
//gazelle/js:tsconfig_paths_test [k8-fastbuild]                         333ms
//gazelle/js:tsconfig_pnpm_ref_rerooted_test [k8-fastbuild]             1s
//gazelle/js:tsconfig_pnpm_ref_test [k8-fastbuild]                      532ms
//gazelle/js:tsconfig_rootdir_test [k8-fastbuild]                       484ms
//gazelle/js:tsconfig_rootdirs_test [k8-fastbuild]                      392ms
//gazelle/js:validate_import_statements_off_test [k8-fastbuild]         309ms
//gazelle/js:validate_import_statements_test [k8-fastbuild]             463ms
//gazelle/js:validate_import_statements_warn_test [k8-fastbuild]        552ms
//gazelle/js:visibility_test [k8-fastbuild]                             382ms
//gazelle/kotlin/parser:parser_test [k8-fastbuild]                      481ms
//gazelle/kotlin:bin_test [k8-fastbuild]                                131ms
//gazelle/kotlin:gazelle_disable_conflict_test [k8-fastbuild]           599ms
//gazelle/kotlin:gazelle_disable_test [k8-fastbuild]                    707ms
//gazelle/kotlin:gazelle_exclude_directive_test [k8-fastbuild]          100ms
//gazelle/kotlin:ignore_config_files_test [k8-fastbuild]                808ms
//gazelle/kotlin:kotlin_test [k8-fastbuild]                             487ms
//gazelle/kotlin:rules_conflicting_name_mapped_kind_test [k8-fastbuild] 221ms
//gazelle/kotlin:rules_conflicting_name_test [k8-fastbuild]             450ms
//gazelle/kotlin:rules_jvm-maven_test [k8-fastbuild]                    159ms
//gazelle/kotlin:simple_file_test [k8-fastbuild]                        110ms
//integration_tests/aspect:configure_test [k8-fastbuild]                42s
//integration_tests/aspect:help_test [k8-fastbuild]                     44s
//integration_tests/aspect:info_test [k8-fastbuild]                     40s
//integration_tests/aspect:version_test [k8-fastbuild]                  33s
//pkg/aspect/build:build_test [k8-fastbuild]                            130ms
//pkg/aspect/lint:lint_test [k8-fastbuild]                              145ms
//pkg/aspect/outputs:outputs_test [k8-fastbuild]                        112ms
//pkg/aspect/run:run_test [k8-fastbuild]                                171ms
//pkg/aspect/test:test_test [k8-fastbuild]                              145ms
//pkg/aspect/version:version_test [k8-fastbuild]                        391ms
//pkg/bazel:bazel_test [k8-fastbuild]                                   916ms
//pkg/plugin/system:system_test [k8-fastbuild]                          235ms
+ 33 other targets

Total test execution time was 8m 33s. 75 tests (36.1%) were fully cached saving 16s.


Buildifier      Format

@jbedard jbedard force-pushed the 788EA150BB0FAAB4A3FF81F26DCBE189 branch 2 times, most recently from 5ca530e to 762f7e7 Compare October 24, 2024 00:49
@jbedard jbedard changed the base branch from main to tools-upgrade October 24, 2024 00:49
@jbedard jbedard force-pushed the 788EA150BB0FAAB4A3FF81F26DCBE189 branch from 762f7e7 to 4f08e7a Compare October 24, 2024 00:50
@jbedard jbedard marked this pull request as draft October 24, 2024 00:51
@jbedard jbedard force-pushed the 788EA150BB0FAAB4A3FF81F26DCBE189 branch from 4f08e7a to 1b38b71 Compare October 24, 2024 00:55
Base automatically changed from tools-upgrade to main October 24, 2024 00:57
@jbedard jbedard force-pushed the 788EA150BB0FAAB4A3FF81F26DCBE189 branch from 1b38b71 to c2ceebe Compare October 24, 2024 00:58
gregmagolan and others added 20 commits October 23, 2024 17:59
…S the exception from Aspect CLI (standard) (#6930)

Aspect CLI Pro is now just Aspect CLI and the open-source, open-core
Aspect CLI is now Aspect CLI OSS.

Also drop the ability to specify the version in
`.aspect/cli/config.yaml` as this is not documented and not used
anywhere currently. Aspect CLI versioning outside of `.bazeliskrc` needs
more thought before we bring it back.

---

- 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

Drop "Pro" from Aspect CLI name. The open-source Aspect CLI OSS now has
the specifier.

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

```
$ bazel-bin/cli/core/cmd/aspect/aspect_/aspect version
Aspect CLI OSS version: unknown [not built with --stamp]

$ bazel-bin/cli/core/cmd/aspect/aspect_/aspect --version
aspect oss unknown [not built with --stamp]

$ ./bazel-bin/cli/pro/pro_/pro version
Aspect CLI version: unknown [not built with --stamp]

$ ./bazel-bin/cli/pro/pro_/pro --version
aspect unknown [not built with --stamp]
```

GitOrigin-RevId: eec53a03fdaa90a8e7401510e50059904de2b349
….<num_commits_in_week>-<commit_hash> (#6932)

Aspect CLI will now be versioned independently from Workflows.

---

### 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

Aspect CLI will now be versioned independently from Workflows. Version
stamp will follow monorepo versioning scheme
`<year>.<week>.<num_commits_in_week>-<commit_hash>`.

### Test plan

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

```
$ bazel-bin/cli/pro/pro_/pro version --aspect:lock_version
Aspect CLI version: 2024.39.51-a6b3d0c7b

$ bazel-bin/cli/pro/pro_/pro --version
aspect 2024.39.51-a6b3d0c7b

$ bazel-bin/cli/core/cmd/aspect/aspect_/aspect version --aspect:lock_version
Aspect CLI OSS version: 2024.39.51-a6b3d0c7b

$ bazel-bin/cli/core/cmd/aspect/aspect_/aspect --version
aspect oss 2024.39.51-a6b3d0c7b
```

GitOrigin-RevId: cc67e6d0d579babe40b8eb43bad3f8d711556914
Minor enhancement to support the .gitignore trailing-/ so it only
matches directories.

---

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

### Test plan

- Covered by existing test cases
- New test cases added

GitOrigin-RevId: 2711ceec5d8d5ffe9ec2f045b7c0994d4b0c5d6c
…js/.d.ts (#6967)

The package.json `main`, `exports` and `types` normally reference the
transpiled js or dts.

Normally imports between .ts are all relative to the source ts so the
`rootDir` and `outDir` aren't taken into account. When the package.json
imports the transpioled .js/dts we must make the transpiled files
available as well.

---

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

### Test plan

- Covered by existing test cases
- New test cases added

GitOrigin-RevId: 0613758a33636c5c8fb1f55425fa7f15ba1b5a04
The `js_library` and `npm_package` targets representing npm packages
were sharing too much code and mixing `npm_package(srcs)` and
`js_library(deps)`.

---

- Covered by existing test cases
- New test cases added

GitOrigin-RevId: 9c03adbceff31bf1aecff18c9ca3373c71513262
Replacing the mix of constants with names and `is*` method names with
only the methods.

The one test change is essentially aligning with the default tsconfig
value where tsx extensions are *not* preserved.

---

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

### Test plan

- Covered by existing test cases

GitOrigin-RevId: 2fef14a6047d026f63abfa44c9612a1b040407ea
tsconfig `rootDir` filtering is now essentially opt-in via a
`${rootDir}` template in the source globs, with the default globs opting
in to keep the default behaviour.

This means we no longer have this confusion about sources being filtered
due to `rootDir` even though we might not be setting
`ts_project(tsconfig, root_dir)` or might even be using `js_library`
when there is nothing to transpile.

This also avoids `filepath.Rel` for every single file, avoids looping
over the files multiple times (collect + filter + group), fetching the
tsconfig for each file etc.

---

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

### Test plan

- Covered by existing test cases
- New test cases added

GitOrigin-RevId: b8e64fc0b5ba7d71fa7cba83d3d4c97510a5fc7c
I have not been able to reproduce this in a standalone test but this
fixes an issue in a client repo.

I think the fix makes sense, the `npm_package(srcs)` attribute may be
set at generation time for source files, but package targets also have
dependencies that must be resolved+merged at resolution time.
Essentially doing `SetAttr("srcs")` at generation and resolution time.

Normally the `srcs` and `deps` are separate attributes, but for
`npm_package` they are the same which is why this is a bit unique.

---

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

### Test plan

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

GitOrigin-RevId: f316224b82d83d69f8858a74017f3e25f77f76bd
Things like package.json might import both the .d.ts and .js version of
a file. Those can come from source .d.ts or .ts.

---

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

### Test plan

- New test cases added

GitOrigin-RevId: 500e11903f791b324a15c05ad2fca9a0a843a058
Feature request from Figma where they have a custom eslint.bzl and
requesting the patches results in two actions on CI.

Also fixes flag registration so they are "no-able" to match expected
behaviour of other flags: `--nofixes`, `--noreport`, etc...

---

- 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

New `--fixes` flag added to lint command in the the Aspect CLI which
defaults to true. Users can now set `--nofixes` to turn off requesting
the lint patch output group (`rules_lint_patch`). Requesting the report
outputs groups (`rules_lint_human` and `rules_lint_machine`) is still
controlled by the `--report` and `--machine` flags.

- Covered by existing test cases
- New test cases added

GitOrigin-RevId: 8351b619b16979d089dd600db9065d8cd48f699d
Close #530

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

### Test plan

- Covered by existing test cases

GitOrigin-RevId: 91c1f483e9b74b668ac7690d52e543bb039f09ac
There are too many edge cases that are causing issues atm. This should
have been optional or opt-in from the start.

This makes these imports "optional", so the feature is still enabled by
default and users upgrading the cli will get this new feature, but it
should no longer ever cause errors. We can maybe change it to error when
opted-in in the future and maybe change the default if we are more
certain it handles most of the edge cases.

---

### 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

Dependencies from package.json `main/types/exports` fields no longer
cause errors when `aspect configure` can not resolve the referenced
files.

### Test plan

- New test cases added

GitOrigin-RevId: 3199133ffd87902ab1906811d5383bb0de56b299
This no longer provides any useful information now that gazelle is
patched to walk subdirectories instead of us doing it manually here.

---

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

### Test plan

- Covered by existing test cases

GitOrigin-RevId: 8cdd9e60d027f509099508b0e0b0a085afc0772d
Various test cases trying to reproduce a customer issue.

---

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

### Test plan

- New test cases added

GitOrigin-RevId: 0e04447116b665e83d89a0e91b9931631f5e44f4
Various test cases trying to reproduce a customer issue.

---

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

### Test plan

- New test cases added

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

### Test plan

- Covered by existing test cases

GitOrigin-RevId: 8f231d868477f4970ec9261edac355f3c157b0b5
…ct deps (#7060)

An issue found at a client when upgrading from an ancient cli version to
more recent.

This changes the order of how dependencies can be resolved, reducing the
precedence of tsconfig paths to be one of the lowest priorities instead
of being on-par with relative file imports.

The client use case and the test case added is when a tsconfig file has
`paths` essentially just for the IDE dev experience or maybe for legacy
non-bazel (or non-pnpm-workspace) reasons. Imports of local workspace
projects should use `:node_modules/@wksp/project-a` via package.json
instead of `//projects/a` vis tsconfig.json.

---

### 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

Pnpm workspace project dependencies resolved with `aspect configure` are
now higher priority then resolving dependencies via tsconfig paths.

### Test plan

- Covered by existing test cases
- New test cases added

GitOrigin-RevId: 7cf0f95489f5600001351d36a7f13500bc465b62
Gitignore and generation-mode are both generic gazelle config and should require no code in plugins.

Note that both of these are currently patched into gazelle while waiting for PRs which
will allow the removal of all aspect code:
bazel-contrib/bazel-gazelle#1921
bazel-contrib/bazel-gazelle#1908

---

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

### Test plan

- Covered by existing test cases

GitOrigin-RevId: cdfe6690ff0ab2584e429419612cc32e94c6b379
…c (#7095)

Package targets must be visible when linked by rules_js so public
visibility is a good default.

Close #758

---

### 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

`configure` generated package targets (`npm_package` or `js_library`)
visibility is now set to public by default.

### Test plan

- Covered by existing test cases

GitOrigin-RevId: 0e6112b2af952a98de0dd6e80339e31bb98089e9
This is a note about the deprecated one (with `js_` prefix) referring to
the new one (no `js_` prefix).

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

### Test plan

- Covered by existing test cases

GitOrigin-RevId: b5a4c1fd7a51cd9b046b7b02978c2f2a709e132b
jbedard and others added 3 commits October 23, 2024 18:00
…--' (#7029)

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

### Test plan

CI

Co-authored-by: Greg Magolan <[email protected]>
GitOrigin-RevId: 72ca500133f72b926bee8df1f476c928ea3ff0cd
Resolves high alerts:
- https://github.com/aspect-build/silo/security/dependabot/99
- https://github.com/aspect-build/silo/security/dependabot/109
- https://github.com/aspect-build/silo/security/dependabot/113

---

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

### Test plan

- Covered by existing test cases

CI

GitOrigin-RevId: 42aea24fd80f10e671935af220d64e2246323788
See bmatcuk/doublestar#92 - each time a glob
is executed it gets validated if it does not successfully match a path.
This now validates it once up front and then avoids re-validating on
every (non-matched) glob invocation.

---

### 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

`aspect configure` glob patterns are now validated once up front instead
of per evaluation.

### Test plan

- Covered by existing test cases

GitOrigin-RevId: 82135c3f6342ea0893d5701b6bb4cfc451e6fe0b
@jbedard jbedard force-pushed the 788EA150BB0FAAB4A3FF81F26DCBE189 branch from c2ceebe to 8376606 Compare October 24, 2024 01:00
@jbedard jbedard marked this pull request as ready for review October 24, 2024 01:12
@jbedard jbedard enabled auto-merge (rebase) October 24, 2024 01:24
@jbedard jbedard merged commit e6863d9 into main Oct 24, 2024
11 checks passed
@jbedard jbedard deleted the 788EA150BB0FAAB4A3FF81F26DCBE189 branch October 24, 2024 01:36
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.

2 participants