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

Centralize dependency logic #975

Merged
merged 9 commits into from
Feb 6, 2020
Merged

Centralize dependency logic #975

merged 9 commits into from
Feb 6, 2020

Conversation

Jamie5
Copy link
Contributor

@Jamie5 Jamie5 commented Jan 30, 2020

Description

We centralize most of the dependency-related logic into one file.

This dependency logic includes

  • Dependency mode (transitive/plus one/direct)
  • unused_deps_mode and strict_deps_mode
  • dependency_tracking_method (currently only high-level, soon ast option as well)
  • what indirect/direct jars/targets we need to compute

In theory, this should not cause any user-facing changes.

We do the following

  • Change collect_jars in scala/private/common.bzl to take information about what is desired (transitivity, direct jars, etc) instead of inferring it from the various settings. Also unify some logic which hopefully makes it clearer what the difference between the various modes are.
  • Add scala/private/dependency.bzl which computes the dependency settings from the various attributes that are provided. Ideally, only this logic (and not the things which call it) will need to change as we move to better +1 deps analysis.
  • Replace the unused deps checker phase with a dependency phase, which includes all dependency information not just unused deps checker info.

Motivation

Part of #867

By centralizing the dependency logic in one place, we will be able to change things around more easily, for when we do start changing the logic to have a better +1 deps analysis.

@Jamie5
Copy link
Contributor Author

Jamie5 commented Jan 30, 2020

@ittaiz this is not necessarily completely polished, would like to see if this kind of approach is the way we should go.

If it is but the diff is too large and confusing I can try to extract out pieces for isolated review, though the isolated pieces may not make too much sense without having the whole in there.

@ittaiz
Copy link
Member

ittaiz commented Jan 30, 2020

I’ll take a look though this is definitely a PR that I’d like to be cautious with because all of the label collection and which jars are passed to the compiler have huge perf potential impact and I want to make sure we’re not missing anything (especially since we don’t have tests about performance).

have a better +1 deps analysis.

What does that mean?

@Jamie5
Copy link
Contributor Author

Jamie5 commented Jan 30, 2020

What does that mean?

Basically the goal of #867 , so that unused deps checker for +1 deps is accurate instead of the existing unused deps checker. (And to also have a strict deps checker for +1 deps)

@ittaiz
Copy link
Member

ittaiz commented Jan 30, 2020

I think I'm missing something so I'd like to verify.
Once the AST mode is merged on the plugin side then your plan is to allow people to opt-in to it and activate it for direct, +1 and indirect? Only for some of them? Are you planning on changing the +1 collection?
Maybe we should do this discussion on the tracking issue?

Copy link
Contributor

@borkaehw borkaehw left a comment

Choose a reason for hiding this comment

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

I am not familiar with dependency code but we (Lucid) will pay more attention when we start focusing on this part. +1 on making smaller PRs for discussion. Thanks for doing this in advance.

scala/private/phases/phase_collect_jars.bzl Outdated Show resolved Hide resolved
scala/private/phases/phase_dependency.bzl Show resolved Hide resolved
@ittaiz
Copy link
Member

ittaiz commented Feb 1, 2020

reviewed (the easy) half.
Hope to finish by tomorrow evening 🤞

@Jamie5
Copy link
Contributor Author

Jamie5 commented Feb 2, 2020

@ittaiz this diff's commit message is also named badly and after merge the first line that people would see would just be calling. I feel like maybe force pushing would cause strange problems so maybe after all the revisions and this is ready to be merged I can do one final force push and get a reasonable commit message? Or do you have an easy way to edit the commit message during squash?

@ittaiz
Copy link
Member

ittaiz commented Feb 2, 2020

Like I mentioned somewhere else- since this has multiple commits the PR title will be the first line of the commit so no worries

Copy link
Member

@ittaiz ittaiz left a comment

Choose a reason for hiding this comment

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

Thanks for this change!
It was very impressive in several areas.
I have to say it was a hard PR to review and partially I think because of formatting changes that maybe would have been better off left to a separate PR. I definitely don't blame it only on that but there were several files where I needed to actually do a side by side comparison of the versions because github's unified as well as split views were so confusing...

Still, thank you!

scala/private/dependency.bzl Outdated Show resolved Hide resolved
scala/private/dependency.bzl Outdated Show resolved Hide resolved
scala/private/dependency.bzl Outdated Show resolved Hide resolved
scala/private/phases/phase_dependency.bzl Show resolved Hide resolved
scala/private/dependency.bzl Show resolved Hide resolved
scala/private/dependency.bzl Outdated Show resolved Hide resolved
scala/private/phases/phase_dependency.bzl Outdated Show resolved Hide resolved
)

# Extract very common code out from dependency analysis into single place
# automatically adds dependency on scala-library and scala-reflect
# collects jars from deps, runtime jars from runtime_deps, and
def _phase_collect_jars(
ctx,
p,
Copy link
Member

Choose a reason for hiding this comment

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

wdyt about passing in p.dependency? p is a very big struct to pass and widens the method (as a reader I'm now more alert)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good

scala/private/phases/phase_compile.bzl Show resolved Hide resolved
scala/private/rule_impls.bzl Show resolved Hide resolved
@Jamie5
Copy link
Contributor Author

Jamie5 commented Feb 3, 2020

@ittaiz sorry about that, I just looked at the split view and yeah it is not very helpful. I think I'm used to this working in split view fine, but will try to avoid similar changes in the future.

@Jamie5
Copy link
Contributor Author

Jamie5 commented Feb 3, 2020

@ittaiz note the pushed changes do not contain the requested comments in _additional_transitive_compile_jars yet, I am just seeing how CI runs with all the other changes

@Jamie5
Copy link
Contributor Author

Jamie5 commented Feb 3, 2020

Ok all comments should now be addressed unless I missed some

def phase_collect_jars_common(ctx, p):
return _phase_collect_jars_default(ctx, p)

def _phase_collect_jars_default(ctx, p, _args = struct()):
return _phase_collect_jars(
ctx,
p.dependency,
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you mind to change it to just p? I know using p.dependency will be less verbose inside the function but just for the purpose of consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change was in response to a comment by @ittaiz . I am fine either way.

Copy link
Member

Choose a reason for hiding this comment

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

@borkaehw why do you think this is a better pattern in general?
Let’s ignore consistency for a sec since maybe we’ll decide to change the pattern

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should keep the function signature as simple as possible. By the fact that p should contain all the information we need, we may expand the data structure within the function instead.

In my opinion,

phase_a(ctx, p, a)
phase_b(ctx, p, a, b)
phase_c(ctx, p, a, b, c, d, e)

make the phase architecture messy. When we finalize #965, I imagine the functions should all be consistent like phase_a(ctx, internal_proviers).

Also, functions with a super long list of arguments like compile_scala makes it hard to read. I, in general, prefer to keep arguments as less as possible and no keyword is needed.

Copy link
Member

Choose a reason for hiding this comment

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

@borkaehw I understand the value in what you say but to be honest I actually quite disagree with it. I think that bottom line having similar APIs is less beneficial than having phases explicitly declare which internal providers they need. Something like providers = [JavaInfo] which we have in startlark.
Having said that I'd rather not start this debate here over this change and so for consistency I apologize @Jamie5 and I'll ask you to revert to your original change. Sorry again.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good. Thanks for sharing your idea.

Copy link
Member

@ittaiz ittaiz left a comment

Choose a reason for hiding this comment

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

Thanks! Sorry for the delay but please see one last comment

def phase_collect_jars_common(ctx, p):
return _phase_collect_jars_default(ctx, p)

def _phase_collect_jars_default(ctx, p, _args = struct()):
return _phase_collect_jars(
ctx,
p.dependency,
Copy link
Member

Choose a reason for hiding this comment

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

@borkaehw I understand the value in what you say but to be honest I actually quite disagree with it. I think that bottom line having similar APIs is less beneficial than having phases explicitly declare which internal providers they need. Something like providers = [JavaInfo] which we have in startlark.
Having said that I'd rather not start this debate here over this change and so for consistency I apologize @Jamie5 and I'll ask you to revert to your original change. Sorry again.

Copy link
Member

@ittaiz ittaiz left a comment

Choose a reason for hiding this comment

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

Thanks!

@ittaiz ittaiz merged commit 88a952d into bazelbuild:master Feb 6, 2020
@Jamie5 Jamie5 deleted the calling branch February 6, 2020 20:37
tian000 pushed a commit to ConsultingMD/rules_scala that referenced this pull request Apr 5, 2020
* Fix code for Bazel change --incompatible_no_support_tools_in_action_inputs (bazelbuild#758)

* Specs2 filtering runner now filters test cases according to filter. (bazelbuild#759)

This allows the bazel test runner correctly generate the test log, based only on tests that actually did run.

* Add scala_doc rule (bazelbuild#760)

* move collect_plugin_paths to common.bzl

* add scala_doc rule + aspect implementations

* add basic scala_doc Markdown documentation

* add scala_doc example

* collect plugins in aspect too

* declare_directory for scaldoc output path or else it complains

* add a simple test target for scala_doc rule

* add doc note about scaladoc being kind of slow

* fix scala_doc.md code block

* privatize scaladoc aspect

* get more src_files/compile_jars

* also accept scalacopts in scala_doc

* turn off scaladoc warnings for now

* use host_path_separator in classpath

* Update scala_doc.md

fix the string `scala_doc` which was copy-pasted as `scala_binary`

* Explicitly convert depset to list (bazelbuild#761)

This will be required by Bazel 0.27, where the flag
`--incompatible_no_support_tools_in_action_inputs` will be on by
default.

The function `collect_plugin_paths` iterates over its argument, so we
need to flatten the depset.

* Make sure that plus-one-deps from exports of direct deps also propagate (bazelbuild#764)

* PlusOne propagates PlusOne deps of exports

* rename of tests

* correct test dependency

* use scala toolchain to toggle on +1 collection

* fix grpc opencensus stats integration (bazelbuild#762)

* fix grpc opencensus stats integration
- upgrade opencensus-java packages to the latest (0.22.1)
- add opencensus-impl and opencensus-impl-core

* add opencensus-impl transitive dependency com.lmax:disruptor

* Add Canva to the adopters list (bazelbuild#770)

* Depset is not iterable (bazelbuild#773)

* Specify which version of bazel is required. (bazelbuild#772)

* Specify which version of bazel is required.

* Update README.md

* Specs2 filtering of JUnit descriptions (bazelbuild#766)

* Specs2 now will create its JUnit Description tree with filtered child items

* Creating a filtered description tree from Specs2 utilities - keeps ordering and hashCodes intact

* Redirecting test error output

* Update "Getting started" WORKSPACE block (bazelbuild#778)

* Migrate from java_common.create_provider to JavaInfo() (bazelbuild#781)

* Migrate from java_common.create_provider to JavaInfo()

* Fix scala/private/common.bzl

* Fix some build failures.

* Fix some more builds.

* Remove scala/private/common:create_java_provider

* Remove unused _create_provider.

* Remove unused load statement

* Also propagate deps and runtime_deps in scala_import with no jars.

* Address review comments.

* Update custom-jvm-rule.bzl

* Update BUILD

* typo

* Removed implicit value for deps_providers.

* Add dummy intermediate jar file for scala_import with no jars.

* Cleanup code.

* Replace + with lists extend.

* Revert enabling --all_incompatible_changes test by mistake.

* Passing source-jar into JavaInfo (bazelbuild#783)

* expose source_jar in JavaInfo

* nit: clarify conditional

* nit: replace one hack with another

* nit: replace concat with append

* remove "main" attr usage

it's no longer needed

* add comment

* remove usage of attr scala with JavaInfo outputs (bazelbuild#784)

* Port bazelbuild/bazel#8196 : improve java_stub_template MANIFEST.MF construction speed (bazelbuild#736)

* Import java_stub_template from bazelbuild@8b8271e

* Port changes from bazelbuild/bazel#8196

* Remove java_stub_template from WORKSPACE

* Update java_stub_template archive URL

* Make java_stub_template a normal file

* JavaInfo provider should be given for deps (bazelbuild#774)

* a JavaInfo provider should be given for deps

* flatten providers lists

* Revert "flatten providers lists"

This reverts commit a464f61.

* remove print warning if dep has no JavaInfo since it's required now (bazelbuild#788)

* Handle FileAlreadyExistsException in ScalaPBGenerator (bazelbuild#789)

* Exclude jmh from reproducibility test, since the code generator is non-deterministic. (bazelbuild#791)

bazelbuild#699

* warn if jvm_flags is used in scala_library (bazelbuild#795)

* Allow for code coverage to be run on 0.27.1 (bazelbuild#780)

* Allow for code coverage to be run on 0.27.1

* Update expected-coverage.dat

* actually remove all merge conflicts

remove scala_doc
merge conflict


spacing


newline

* Replace jar_sha256 with artifact_sha256. (bazelbuild#792)

* Simplify _jacoco_offline_instrument. (bazelbuild#790)

resolve_command shouldn't be required here.

* silence tut (bazelbuild#799)

* Due to limitations in the toolchains implementation this is required … (bazelbuild#801)

* Add test for host deps

* Add a test hopefully to illustrate host deps

* Update test

* Change api usage to use binds

* Remove errant print

* See if behavior is different on 0.28.1

* incompatible_string_join_requires_strings: string.join(list) accepts invalid (non-string) list elements

* Add a to_list

* Another case of depset iterable

* Windows ci can only support 0.28.0 via chocolaty right now

* Add scalac_jvm_flags option to toolchain (bazelbuild#803)

* Add scalac_jvm_flags to scala_toolchain

This allows things like setting the max heap on all Scalac workers.

* Add docs

* Fix comment

* Add enable_code_coverage_aspect to the docs

* Flags on target should override flags on toolchain.

Also fix comment.

* Add `scala_test_jvm_flags` to toolchain (bazelbuild#804)

* Add scala_test_jvm_flags to the toolchain

* Fix package name

* Fix target names

* Add trivial test suite and rename some things

* Wrap all jvm_flags in _expand_location

* Remove the deprecated attribute proto_source_root. (bazelbuild#793)

* Remove the deprecated attribute proto_source_root.

Replace it with strip_import_prefix, its spiritual successor.

* Update Bazel version on Travis

* Update rules_scala to work with Bazel >=0.27.

The flag --incompatible_string_join_requires_strings was flipped, which
this repository was incompatible with.

* Update to Bazel 0.26 instead.

test_coverage_on fails for some mysterious reason that seems unrelated to the cleanup crusade I'm pursuing at the moment.

* add point release number so that downloading Bazel succeeds

* change whitespace to re-trigger build

* update Bazel version, hopefully properly

* update test_expected_failure/

* minimize diff

* re-trigger Travis

* re-trigger Travis again

* update README.md

note the last version of 0.23 that we ran CI on.

* Fix a regression that breaks scalapb when jvm_flags are set on the toolchain (bazelbuild#806)

* Fix a regression that breaks scalapb when jvm_flags are set on the toolchain

* Move passing manual tests to new directory

* Migrate from old-style .java provider to JavaInfo. (bazelbuild#807)

* Migrate from old-style .java provider to JavaInfo.

* Remove usage of .scala.

* Flip these around to be correct (bazelbuild#810)

* Clean up jmh rule a bit (bazelbuild#811)

* Move scala_repositories macro out to its own file + move scala_doc.bzl (bazelbuild#808)

* move scala_repositories macro out to its own file

* move scala_repositories.bzl and scala_doc.bzl to private

* Pin Bazel versions (bazelbuild#812)

* Pin Bazel versions

* ensure one set of jobs is named test

* Keep the original build structure

* fix version for windows

* add ci stopgap to scripts used in ci

* fix ci errors

* adjust for buildkite

* remove unused param

* Tweak travis image config

* use sh language

* libxml2-utils

* use apt addon

* Add notes about updating bazel versions

* Move scala_binary to its own file (bazelbuild#813)

* add a note about code organization in CONTRIBUTING.md

* wip move scala_binary out

* fully split out scala_binary properly

* _library_outputs is the same thing as common_outputs

* fix a bunch more scalac_provider references

* rename scala_provider function to get_scalac_provider per review

* back to the old variable names

* Delurk Powershell for a more unified test setup (bazelbuild#814)

* Attempt to delurk powershell

* 💥

* adjust

* adjust

* Add a basic PR template (bazelbuild#817)

* Move scala_test/scala_test_suite to their own file (bazelbuild#815)

* move scala_test rule to its own file

* move scala_test_suite to scala_test.bzl, move sanitize_string_for_usage function to common.bzl for now

* add a docstring about scala_test.bzl

* move test_resolve_deps to private variable in scala_test.bzl

* remove suites attribute debug print

* move scala_repl rule to its own file (bazelbuild#820)

* remove long-deprecated 'suites' attr in scala_test (bazelbuild#819)

* move scala_junit_test rule to its own file (bazelbuild#822)

* rename proto rule while maintaining backwards compat (bazelbuild#821)

* Disable windows CI (bazelbuild#823)

* Move library rules (bazelbuild#827)

* move library_attrs to common_attributes.bzl

* move scala_macro_library rule to its own file

* move all _library rules to scala_library.bzl, private stuff too

* move _lib into scala_library.bzl

* alphasort

* Buildifier as the only lint (bazelbuild#826)

* Load buildifier directly

* update lint scripts

* let buildifier reformat everything

* Test lints in CI

* remove accidental file extension

* use skylib version compatible with rules_go and buildifier

* fix an unformatted file that breaks ci (bazelbuild#830)

* use travis job pipelines (bazelbuild#829)

* Default to usage of scala_proto_library instead of scalapb_proto_library alias (bazelbuild#831)

* Refactor build_deployable (bazelbuild#832)

* build_deployable -> merge_jars with a more explicit interface

* add docstring doc to merge_jars

* buildifier fix

* parameterize the entire progress_message

* Update README.md to include Grand Rounds (bazelbuild#835)

* Minor fix to error message (bazelbuild#841)

Was not properly adding space, as such:

```
java.lang.IllegalStateException: Was not able to discover any classes for archives=testsuite/test/example/example.jarprefixes=Set(), suffixes=Set(Test)
```

* Remove jvm_flags debug print for scala_library (bazelbuild#840)

* remove jvm_flags debug print for scala_library

* hard fail for jvm_flags in scala_library, re-add jvm_flags attr for other rules

* remove fail, not needed if attr isn't supported

* Improve classpath manifest file construction (bazelbuild#842)

* use java_common.merge instead of manual _collect util functions (bazelbuild#838)

* Fix paths in --proto_path and avoid copying proto files (bazelbuild#850)

* Fix paths in proto_path and avoid copying

* Prepare mapping in advance

* Condensed all transformations into one method

* Added tests

* Buildifier corrections

* Split sh test (bazelbuild#849)

* Split shell ingetration tests

* Fix test_repl no target from clean build

* Fix scala_library_jar_without_srcs_must_fail_on_mismatching_resource_strip_prefix (bazelbuild#859)

* Fix test_scala_import_source_jar_should_not_be_fetched_when_env_bazel_jvm_fetch_sources_is_set_to_non_true (bazelbuild#858)

* Add a test build for bazel 1.0 (bazelbuild#861)

* Add a test build for bazel 1.0

* cp

* cp

* I think i messed this up a little and it wasn't running the older ones too, making sure we run everything this time

* cp

* include Bazel 1.0.0 in compatibility table (bazelbuild#863)

* fix typo in codeowners github handle

* Mirror all http_archives. (bazelbuild#878)

Context: bazelbuild/bazel#10270

* Bump v1.0.0 compatibility test to v1.1.0 (bazelbuild#882)

* Bump v1.1.0 compatibility test to v1.2.1 (bazelbuild#883)

* Bump v1.1.0 compatibility test to v1.2.0

* Upgrade MacOS from HighSierra to Mojave

* Empty commit to trigger a new build

* Bump bazel to v1.2.1

* Fix sha for 0.28.0

* Revert "Upgrade MacOS from HighSierra to Mojave"

This reverts commit a239d4e.

* Update sha for 0.28.0 to HEAD

* Explicitly label bazel 0.28.1 (bazelbuild#885)

* Bump scala 2.12 to v2.12.10 (bazelbuild#886)

* Convert maven_jar to jvm_maven_import_external (bazelbuild#889)

* Bump Bazel to v2.0.0 (bazelbuild#902)

* authorship attribution for higherkindness/rules_scala design (bazelbuild#903)

* Refactor rules into configurable phases (bazelbuild#865)

* Add configurable phases

* Refactor rules implementation into configurable phases

* Customizable phases

* Customizable phases tests

* Break up init to more reasonable phases

* Move final to non-configurable phase

* Rename parameter builtin_customizable_phases

* Fix ijar

* Switch default for buildijar

* Add TODOs

* Rename provider

* Move to advanced_usage

* rename custom_phases

* Make default phase private

* Fix exports_jars

* Adjusted_phases

* Rename p to be more clear

* Add in-line comments

* Fix lint

* Add doc for phases

* Doc for consumers

* Doc for contributors

* Add more content

* Fix md

* Test for all rules

* Fix junit test

* Fix lint

* Add more tests

* Fix junit test

* Fix doc

* Change _test_ to _scalatest_

* More doc on provider

* expand locations in scalacopts (bazelbuild#890)

* expand locations in scalac options

* allow plugins in expansion

* add a happy path test

* make the target names more obvious

* comment

* Revert "expand locations in scalacopts (bazelbuild#890)" (bazelbuild#904)

This reverts commit 5c966ee.

* expand locations in scalacptops, take 2 (bazelbuild#907)

* expand locations in scalacopts (bazelbuild#890)

* expand locations in scalac options

* allow plugins in expansion

* add a happy path test

* make the target names more obvious

* comment

* access ctx.attr.plugins with fallback

* reformat

* Plugin expansion- Use input plugins param instead of ctx (bazelbuild#909)

* See test failing

* Use input plugins param instead of ctx

* Remove phase scala provider (bazelbuild#913)

* phase_jvm_flags uses JavaInfo provider instead of scala_provider

* remove phase scala_provider

* readme clarifies minimum version at HEAD is 1.1.0

* travis.yml moved from 0.28.1 to 1.1.0

* Use https to access central.maven.org (bazelbuild#920)

See https://support.sonatype.com/hc/en-us/articles/360041287334
And use repo.maven.apache.org instead of central.maven.org

* remove me from codeowners

I'm not currently a maintainer of this project.

* Return providers array instead of legacy struct (bazelbuild#916)

* runfiles and files are part of explicit DefaultInfo provider
and do not come from attributes

* removed transitive_rjars attribute as it was only needed internally
and before phases was exposed mistakenly because that's how the infra worked
Now internally phases use p.compile.rjars

* executable attribute part of DefaultInfo as well

* use coverage_common.instrumented_files_info provider instead of attribute

* remove redundant attributes

* linting

* return array of providers instead of struct

* scala_import return array of providers instead of struct

* Move declare_executable to phase file (bazelbuild#919)

* chore(docs): update WORKSPACE setup for skylib (bazelbuild#926)

* scalatest final-remove redundant coverage handling (bazelbuild#923)

* update bazel-toolchains version (bazelbuild#937)

fix bazelbuild#901

* Move code from rule impls/common to phases (bazelbuild#930)

* collect_srcjars to phase

* move compile_or_empty and needed code to phase_compile (from rule_impls)

* phase_compile to take scalac_provider from phases instead of rule_impls

* rule_impls loads are loaded as private and unused are removed

* get_scalac_provider in phase_scalac_provider and not rule_impls

* move write_java_wrapper from rule_impls to phase_java_wrapper

* move merge_jars from rule_impls to phase_merge_jars

* move get_unused_dependency_checker_mode from rule_impls to get_unused_dependency_checker_mode

* move write_manifest from common to get_write_manifest

* move collect_jars_from_common_ctx from rule_impls to phase_collect_jars

* move write_executable from rule_impls to phase_write_executable

* linting

* [CR] inline _collect_jars_from_common_ctx

* [CR] inline _collect_srcjars

* [CR] inline write_java_wrapper

* [CR] inline merge_jars

* [CR] inline _write_executable

* use correct delimiter for joined values (bazelbuild#941)

* deploy_jar creation uses args file to support long classpaths (bazelbuild#942)

* deploy_jar creation uses args file to support long classpaths

* allow bazel to not always use param file

* linting

* exposing java jar to BEP from all binary rules (bazelbuild#943)

* exposing java jar to BEP from all binary rules
added a passing e2e for java jar of scala library for symmetry

* simplify test setup

* remove ctx.outputs.jar usage from phase_library_final (exists in p.compile.full_jars)

* linting

* remove unused method (bazelbuild#945)

* Fix resource_strip_prefix of scala_libary for external repositories (bazelbuild#944)

* Fix resource_strip_prefix of scala_libary for external repositories

If repository `A` has a `scala_library` with `resources` and
`resource_strip_prefix` on which repository `B` depends then repository
`B` fails to build complaining that resource doesn't start with correct
prefix

* Push prefix handling down to _adjust_resources_path_by_strip_prefix
Use skylib to concat paths

* Rewrite test with the external repo
This reflects situation described in PR

* Run buildifier

* Revert external repository

* Revrite test with local_repository

* Revert visiblity of scala_library

* Custom phases expose custom providers (bazelbuild#946)

* rules_scala custom phases should expose providers- failing test

* rules_scala custom phases should expose providers- passing test
while doing it I found and fixed a bug in phases adjustments of first/last (they were duplicated)
Additionally this mandates bazel 2.0.0 as they have some kind of bug with providers before that (don't have an issue to ref but couldn't get the build to pass with lower bazel version)

* update readme to note when we stopped fully supporting 1.1.0

* remove unneeded 1.2.1 shas

* drop 1.1.0 travis builds

* remove 1.1.0 from bazel wrapper

* break after adding positional phase

* rule_providers to external_providers

* Remove unused reference to compile_scala (bazelbuild#950)

* final phase is inlined (bazelbuild#947)

all phases are equal
DefaultInfo is returned by a default_info phase
Other external providers are passed by their respective phases

* Rename phases logically (bazelbuild#953)

* Phases can override providers from previous phases (bazelbuild#948)

* override providers- failing test

* override providers- passing test

* docs

* Bundle input protos together with generated/compiled classes (bazelbuild#949)

* Init

* Pass bazel test //test/... with single _adjust_resources_path

* adjust_resources_path returns string instead of tuple

* remove prefix handling from ScalacProcessor.copyResources

* Remove unused method

* Remove resourceStripPrefix from CompileOptions

* Remove special path handling from ScalacProcessor because it is done in rule_impls.bzl

* Fix macro visibility

* Remove print

* Formatting

* Add comments

* more test cases

* lint

* Rebase on master

* extract resources.bzl

* Merge dependency_analyzer and unused_dependency_checker (bazelbuild#954)

* Merge dependency_analyzer and unused_dependency_checker

We merge dependency_analyer and unused_dependency_checker
projects because of the code they have in common, and
because we will be adding additional forms of checking
which would be complex with two different projects
going on.

* split

* ignore the .metals dir created by VSCode/Emacs with metals (bazelbuild#957)

* Fix doc for new naming convention (bazelbuild#960)

* Update ij.bazelproject (bazelbuild#961)

* Phase Scalafmt (bazelbuild#912)

* Phase Scalafmt

* Reuse formatter

* Remove glob

* Remove rules_jvm_external

* Rename argparse

* Remove executable

* Use shared code

* Remove imports

* Add comment

* Change file name

* Move args to private function

* Change to true

* Change conf location

* Change default conf

* Test custom conf

* Fix lint

* Fix build

* Remove trailing commas

* Add formatted and unformatted folder

* Rename test function

* Remove template file

* Better handle failing case

* Drop argparser

* Remove resolve_command

* Add comments

* Remove unnecessary code

* Change to RUNPATH

* Rename gitignore backup

* Remove comment

* Move conf file

* Add doc to attribute

* Switch to match readme

* Add doc

* Move doc to separate md

* Fix wording

* Fix lint

* Add url

* Add url

* use worker proto file instead of precompiled java (bazelbuild#956)

* Unify all maven urls (bazelbuild#968)

* readme (bazelbuild#969)

* fix proto regression (bazelbuild#963)

* add the cross repo proto test in the manner of $WORK usage

* conditionally trim root path based off of overall stripped prefix

* just an empty commit to trigger a CI rebuild

* simplify test case

* Add defs.bzl with core rules and notice about evolving APIs (bazelbuild#955)

* Add defs.bzl with core rules

* move to unstable directory

* unify all of the default info phases implementations (bazelbuild#958)

* unify all of the default info phases implementations

* collect_data = True

* Fix maven server urls (bazelbuild#973)

* Simplify phase_compile return struct (bazelbuild#974)

* Add a per-scala-version test for dependency analyzer (bazelbuild#971)

* multi_version

* change

* empty

* remove unused method (bazelbuild#976)

* Get files with extension (bazelbuild#979)

* Get files helper function

* Remove extra computation

* Move extension to shared file

* Move functions to paths.bzl

* Use skylib path is_absolute (bazelbuild#982)

* Add AST walking mode for dependency analyzer (bazelbuild#967)

* plugin

* lint

* Separate coverage (bazelbuild#972)

* Separate coverage from compile

* Remove unused

* Simplify condition

* Simplify struct

* Simplify return statement

* Simplify phase

* Fix lint

* Remove condition

* plugin3 (bazelbuild#986)

* Handle inlined literals in AST walker (bazelbuild#985)

* Handle inlined literals in AST walker

* comments

* comments

* When building JMH benchmarks, fail on error, instead of proceeding. (bazelbuild#977)

* When building JMH benchmarks, fail on error, instead of proceeding.

Currently, if there are errors in some (but not all) benchmarks, running
`bazel run //path/to/benchmark` will compile them, fail on some, and
then run the rest.

This changes that behavior so that any JMH build failure will fail the
build.

* Add a test for expected failures in JMH benchmarks.

* Document the fix to JMH builds as a breaking change.

* Split "test_benchmark_jmh_failure" from "test_benchmark_jmh".

* We don't need ValidBenchmark.scala when testing JMH failures.

* Centralize dependency logic (bazelbuild#975)

* calling

* lint

* path

* comments

* comments

* lint

* comment

* Include location in error report (bazelbuild#988)

* Include location in error report

* add test

* Ignore scalatest in unused dependency checker (bazelbuild#990)

* Ignore scalatest in unused dependency checker

* lint

* Make rules_scala compatible with --incompatible_load_proto_rules_from_bzl (bazelbuild#989)

* Make rules_scala compatible with --incompatible_load_proto_rules_from_bzl

* Use [email protected]

* Run ./lint.sh

* Add load() for proto_library to @proto_cross_repo_boundary (bazelbuild#996)

Missed that one in bazelbuild#989.

* Update dependency configuration options (bazelbuild#995)

* Update dependency configuration

* lint

* comments

* comment

* comments

* Ignore base class annotation (bazelbuild#991)

* load java rules explicitly (bazelbuild#999)

* load python rules explicitly (bazelbuild#1000)

* Add test for scalac dependencies (bazelbuild#998)

* Add test for scalac dependencies

* comment

* specify deps for specs2 and specs2-junit deps  (bazelbuild#1002)

* specify deps for specs2 deps

needed for strict-deps/unused-deps

* Update specs2.bzl

* Update specs2_junit.bzl

* Update specs2_junit.bzl

* Update aspect.bzl

* Update aspect.bzl

* remove spotify from adopters (bazelbuild#1001)

They looked at Bazel but didn't adopt

* Revert "remove spotify from adopters (bazelbuild#1001)" (bazelbuild#1003)

This reverts commit 6442e78.

* Scalafmt 2.12 support (bazelbuild#1004)

* Scalafmt 2.12 support

* Upgrade scalafmt version

* Fix source pathing for code coverage (bazelbuild#1006)

* Fix source pathing for code coverage

* lint fixes hopefully?

* lint? idk what's going on here

* Rename in_out_pair to records

* Remove bad assumptions about how bazel works

* Clean up lingering srcs

Clean up lingering srcs

* this commit should fail tests

* this commit should pass tests

* let this test actually fail the CI

* update comments

* Refactor src_paths

* spelling is hard

* update to the latest worker protobuf file (bazelbuild#1007)

* Report macros as used (bazelbuild#1008)

* Update README.md with protobuf version matching actual dependencies (bazelbuild#1012)

Version taken from scala/private/macros/scala_repositories.bzl.
Previous version from docs was conflicting with actual version of zlib.

* Don't unnecessarily use fullyExploreTree (bazelbuild#1009)

* Fixed protobuf URL in README (bazelbuild#1013)

* Return resource's short_path when fail to find relative (bazelbuild#1010)

* Return resource's short_path when fail to find relative

* Add tests for imports (bazelbuild#1016)

* Add tests for imports

* update

* use bind to remove loader-specific labels in dependencies (bazelbuild#980)

* use bind to remove loader-specific labels in dependencies

* remove redundant bind

* Documentation for coverage support (bazelbuild#1017)

* Documentation for coverage support

* Coverage: update example script to use combined coverage report

* Coverage docs: adding note about only being tested with ScalaTest

* toolchain which turns on coverage is part of API (bazelbuild#1020)

* update rules_scala version (bazelbuild#1022)

fixes bazelbuild#1021

* Add fetch_source parameter to scala_repositories (bazelbuild#1027)

* add fetch sources to scala_repositories

* adding fetch_sources to WORKSPACE

Co-authored-by: Laurent Le Brun <[email protected]>
Co-authored-by: Igal Tabachnik <[email protected]>
Co-authored-by: Long Cao <[email protected]>
Co-authored-by: P. Oscar Boykin <[email protected]>
Co-authored-by: Ittai Zeidman <[email protected]>
Co-authored-by: Mackenzie Starr <[email protected]>
Co-authored-by: Jonathon Belotti <[email protected]>
Co-authored-by: ianoc-stripe <[email protected]>
Co-authored-by: Parth Mehrotra <[email protected]>
Co-authored-by: Christopher Johnson <[email protected]>
Co-authored-by: Irina Iancu <[email protected]>
Co-authored-by: Ittai Zeidman <[email protected]>
Co-authored-by: joshrosen-stripe <[email protected]>
Co-authored-by: Paul Tarjan <[email protected]>
Co-authored-by: Benjamin Peterson <[email protected]>
Co-authored-by: David Haxton <[email protected]>
Co-authored-by: Alex Beal <[email protected]>
Co-authored-by: lberki <[email protected]>
Co-authored-by: Andy Scott <[email protected]>
Co-authored-by: Mantas Sakalauskas <[email protected]>
Co-authored-by: Shachar Anchelovich <[email protected]>
Co-authored-by: ignasl <[email protected]>
Co-authored-by: Bor Kae Hwang <[email protected]>
Co-authored-by: Bor Kae Hwang <[email protected]>
Co-authored-by: Jonathon Belotti <[email protected]>
Co-authored-by: Philipp Wollermann <[email protected]>
Co-authored-by: chenrui <[email protected]>
Co-authored-by: chenrui <[email protected]>
Co-authored-by: Jin <[email protected]>
Co-authored-by: Andreas Herrmann <[email protected]>
Co-authored-by: rivashin <[email protected]>
Co-authored-by: Simonas Pinevičius <[email protected]>
Co-authored-by: Jamie5 <[email protected]>
Co-authored-by: Jamie5 <[email protected]>
Co-authored-by: Samir Talwar <[email protected]>
Co-authored-by: Yannic <[email protected]>
Co-authored-by: David Haxton <[email protected]>
Co-authored-by: Gergely Fábián <[email protected]>
Co-authored-by: pawelaw <[email protected]>
Co-authored-by: Steven Parkes <[email protected]>
SocksDevil pushed a commit to SocksDevil/rules_scala that referenced this pull request Jul 6, 2020
* calling

* lint

* path

* comments

* comments

* lint

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

Successfully merging this pull request may close these issues.

4 participants