unify all of the default info phases implementations #958

andyscott · 2020-01-27T00:56:13Z

Description

Unifies all of the default_info phases into a single implementation.

Motivation

This makes it relatively straightforward for any phase to augment the files and runfiles DefaultInfo output. This is extremely useful when adding new phases for special functionality. In my case, I need functionality to support a new test discovery phase that writes a list of tests to a text file output.

andyscott · 2020-01-27T00:57:12Z

scala/private/phases/phase_runfiles.bzl

@@ -38,7 +38,7 @@ def _phase_runfiles_default(ctx, p, _args = struct()):
    return _phase_runfiles(
        ctx,
        _args.transitive_files if hasattr(_args, "transitive_files") else depset(
-            [p.declare_executable, p.java_wrapper] + ctx.files._java_runtime,


Last I check, the DefaultInfo's executable field is assumed part of the overall runfiles output. So this wasn't needed here.

Do you think we can validate it somehow? Do you think tests should have been broken? Maybe it's good enough, not sure

IMHO it's good enough because otherwise most of the tests covering binary and test rules would have broken! Additionally, I wouldn't have been able to iterate on #959 which incorporates these changes.

If you want, I can test this against our codebase at Stripe to be extra safe.

I think that’s a good idea (though you’re probably right)

andyscott · 2020-01-27T00:57:25Z

scala/private/phases/phase_runfiles.bzl

@@ -24,7 +24,7 @@ def phase_runfiles_scalatest(ctx, p):

    args = struct(
        transitive_files = depset(
-            [p.declare_executable, p.java_wrapper] + ctx.files._java_runtime + runfiles_ext,


Same as the below comment.

ittaiz

Thanks. I didn't go over the few important details this PR has but in general it looks like this ended up being about two things: Unification and Dependency inversion (instead of default_info taking fields from phases you're ignoring the phase and using the field name).
The unification looks good in general (will take a look at details later) and it was an annoyance when I looked at it so nice one :)
Re dependency inversion- My plan (which I shared in a few PRs/Issues) is actually to kill p in favor of a new field internal_providers which will be a dict with Bazel providers which phases expose.
The fact that phases expose providers will allow clearer understanding of their semi-public API (right now you need to go and rummage through the phases to see what exactly goes on and also when the API changes it's more implicitly). In addition providers with dict can allow overrides and customizations of following phases like the external_providers is doing.
One thing I didn't get to implement with external_providers is to actually pass external_providers as an input so phases can copy and not only create completely new ones.

To summarize I'd love it if we could take this PR and change it so that actually we have 3 new internal providers (FilesToExpose, RunFilesToExpose, ExecutableToExpose) or maybe 1 new internal provider (DefaultInfoToExpose) that the phases take as input and can return.
Then the default_info phase can just take that from the dict.

WDYT? If it's not 100% clear I can send this PR (with the changes here) to clarify how I see this pattern.

ittaiz · 2020-01-27T05:40:12Z

scala/private/phases/phase_compile.bzl

@@ -165,6 +165,7 @@ def _phase_compile(
        class_jar = out.class_jar,
        coverage = out.coverage.external,
        full_jars = out.full_jars,
+        files = depset(out.full_jars),


having a depset here is probably not the best idea since depsets have their cost and especially since you want to customize this in other phases then you might have depsets which are nested for no reason.
Maybe pass array?

I'm not sure. In some case we may wind up passing along existing depsets, but in many others we have just a few files that need to be added in. It's not clear to me which is more common and the impact to analysis time.

Are you replying in general or about files and runfiles? What we have now in these cases are arrays. If you’ll want to override these then you’ll need to have a nested depset.
In general I think it will depend on the case.

ittaiz · 2020-01-27T05:40:46Z

scala/private/phases/phase_runfiles.bzl

@@ -38,7 +38,7 @@ def _phase_runfiles_default(ctx, p, _args = struct()):
    return _phase_runfiles(
        ctx,
        _args.transitive_files if hasattr(_args, "transitive_files") else depset(
-            [p.declare_executable, p.java_wrapper] + ctx.files._java_runtime,


Do you think we can validate it somehow? Do you think tests should have been broken? Maybe it's good enough, not sure

andyscott · 2020-01-27T08:02:50Z

I'm off to bed. I'll respond to the overall comment tomorrow.

andyscott · 2020-01-28T08:10:06Z

I haven't been able to check this against Stripe's codebase due to the pending fix in #963.

I'd love to see this merged to get the consolidation of the default info methods. We can then have a separate discussion about cleaning up how we pass information between phases.

But since we're here, I'm not convinced that providers are the way to go for passing phase information. Providers are typically used for passing information across rule boundaries: rule implementation a -- target graph --> rule implementation b. Here we are passing information between functions within the same rule implementation, i.e. phase 1 --> phase 2, all within rule implementation c. Perhaps we should just use well structured structs or dictionaries?

ittaiz · 2020-01-28T15:21:01Z

This is adding tech debt (passing info via p) which I've been very vocal against for a few months now. We've been postponing this and keep saying that we don't want to block efforts but this can't hold a lot further. I voiced my concern about the contract between phases being too vague in a few places, namely in the big phases PR.
This PR actually hits this issue and then works around it via iteration.

I can merge this (even though I really don't like it) but then we need to stop and agree on the pattern (iteration [I don't like it but am open to hear arguments in favor of it], providers or structs/dictionaries).
WDYT?
(also see #965 for the continuation of the discussion)

andyscott · 2020-01-28T18:16:05Z

The tech debt is already there, this just changes it a bit 😄. I agree with you on narrowing down the contract between phases. Part of that process is establishing what we need phases to do, and this PR demonstrates a need for a generic way to accumulate values for returned providers. Basically we need some monoial computation to thread through the phases. I'll follow up on the issue, and perhaps do a few proof of concepts. I'd prefer we merge this though as ties into some other work, and I like the cleanup of the default info methods.

…

On Tue, Jan 28, 2020, 07:21 Ittai Zeidman ***@***.***> wrote: This is adding tech debt (passing info via p) which I've been very vocal against for a few months now. We've been postponing this and keep saying that we don't want to block efforts but this can't hold a lot further. I voiced my concern about the contract between phases being too vague in a few places, namely in the big phases PR. This PR actually hits this issue and then works around it via iteration. I can merge this (even though I really don't like it) but then we need to stop and agree on the pattern (iteration [I don't like it but am open to hear arguments in favor of it], providers or structs/dictionaries). WDYT? (also see #965 <#965> for the continuation of the discussion) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#958?email_source=notifications&email_token=AACLYW5LPXBH75HYYFKSYHLRABEN5A5CNFSM4KL2LFNKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKDV4OQ#issuecomment-579296826>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACLYW2ZEGDCM6KCN7WJUFDRABEN5ANCNFSM4KL2LFNA> .

ittaiz · 2020-01-28T19:15:21Z

Ok, let’s merge and then see how we agree on something on the issue before we do any more changes (I was about to send a series of PRs to move us to providers so I’m blocked until we decide).
Since you need to rebase and resolve conflicts, Wdyt about what I said about the depset? In these concrete instances it sounds like an array is the better solution

andyscott · 2020-01-29T00:16:26Z

I tried converting the depsets to an array but we already have a lot of existing code managing the transitive_files in the runfiles phase as a depset. We'll have to call an extra to_list or do more extensive rewriting to use arrays in this PR.

smparkes · 2020-01-30T03:14:30Z

comment moved to #965

ittaiz · 2020-01-30T04:04:33Z

@smparkes can you move this comment to #965 ? I have a reply but I’d like to have a clear discussion page about it

smparkes · 2020-01-30T04:59:48Z

@ittaiz done

* 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 rules_java@0.1.1 * 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 <laurentlb@gmail.com> Co-authored-by: Igal Tabachnik <igalt@wix.com> Co-authored-by: Long Cao <48221800+long-stripe@users.noreply.github.com> Co-authored-by: P. Oscar Boykin <johnynek@users.noreply.github.com> Co-authored-by: Ittai Zeidman <ittaiz@wix.com> Co-authored-by: Mackenzie Starr <mackestarr@gmail.com> Co-authored-by: Jonathon Belotti <jonathon@canva.com> Co-authored-by: ianoc-stripe <ianoc@stripe.com> Co-authored-by: Parth Mehrotra <parth.mehrotra.cs@gmail.com> Co-authored-by: Christopher Johnson <tenorviol@yahoo.com> Co-authored-by: Irina Iancu <iirina@users.noreply.github.com> Co-authored-by: Ittai Zeidman <ittaiz@gmail.com> Co-authored-by: joshrosen-stripe <48632449+joshrosen-stripe@users.noreply.github.com> Co-authored-by: Paul Tarjan <github@paulisageek.com> Co-authored-by: Benjamin Peterson <benjamin@python.org> Co-authored-by: David Haxton <haxton.dc@gmail.com> Co-authored-by: Alex Beal <39505601+beala-stripe@users.noreply.github.com> Co-authored-by: lberki <lberki@users.noreply.github.com> Co-authored-by: Andy Scott <andyscott@users.noreply.github.com> Co-authored-by: Mantas Sakalauskas <mantass@wix.com> Co-authored-by: Shachar Anchelovich <shacharan@wix.com> Co-authored-by: ignasl <ignasl@wix.com> Co-authored-by: Bor Kae Hwang <borkaehw@umich.edu> Co-authored-by: Bor Kae Hwang <borkaehw@lucidchart.com> Co-authored-by: Jonathon Belotti <jonathon.bel.melbourne@gmail.com> Co-authored-by: Philipp Wollermann <philwo@google.com> Co-authored-by: chenrui <rui@meetup.com> Co-authored-by: chenrui <chenrui333@gmail.com> Co-authored-by: Jin <jin@users.noreply.github.com> Co-authored-by: Andreas Herrmann <andreas.herrmann@tweag.io> Co-authored-by: rivashin <rivashin@gmail.com> Co-authored-by: Simonas Pinevičius <simonasp@wix.com> Co-authored-by: Jamie5 <JamieLiPanda@gmail.com> Co-authored-by: Jamie5 <jamieli550@gmail.com> Co-authored-by: Samir Talwar <samir@noodlesandwich.com> Co-authored-by: Yannic <contact@yannic-bonenberger.com> Co-authored-by: David Haxton <davidhaxton@asana.com> Co-authored-by: Gergely Fábián <gergo.fb@gmail.com> Co-authored-by: pawelaw <pwronisz@gmail.com> Co-authored-by: Steven Parkes <steven.parkes@ascend.io>

* unify all of the default info phases implementations * collect_data = True

This fixes a bug introduced by bazelbuild#958. Before bazelbuild#958, `rootpath` on a Scala binary resolved to the wrapper binary so other rules could treat it as an opaque executable. After that PR `rootpath` now resolves to the JAR instead of the wrapper binary. Adding the executable back as a direct dependency fixes that.

* Add executable as a direct dependency This fixes a bug introduced by #958. Before #958, `rootpath` on a Scala binary resolved to the wrapper binary so other rules could treat it as an opaque executable. After that PR `rootpath` now resolves to the JAR instead of the wrapper binary. Adding the executable back as a direct dependency fixes that. * Add a testcase

unify all of the default info phases implementations

082f815

andyscott requested a review from ittaiz as a code owner January 27, 2020 00:56

googlebot added the cla: yes label Jan 27, 2020

andyscott commented Jan 27, 2020

View reviewed changes

collect_data = True

8f63bb6

andyscott mentioned this pull request Jan 27, 2020

Generic scala_test rule #959

Closed

ittaiz reviewed Jan 27, 2020

View reviewed changes

ittaiz mentioned this pull request Jan 27, 2020

Phase Scalafmt #912

Merged

1 task

ittaiz mentioned this pull request Jan 28, 2020

Information passing between phases #965

Open

Merge branch 'master' into andyscott/unify-default-info-phase

9410cc3

ittaiz merged commit 96f9d94 into bazelbuild:master Jan 29, 2020

SocksDevil pushed a commit to SocksDevil/rules_scala that referenced this pull request Jul 6, 2020

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

8b4a85a

* unify all of the default info phases implementations * collect_data = True

cocreature mentioned this pull request Jul 27, 2020

Add executable as a direct dependency #1082

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

unify all of the default info phases implementations #958

unify all of the default info phases implementations #958

andyscott commented Jan 27, 2020

andyscott Jan 27, 2020

ittaiz Jan 27, 2020

andyscott Jan 27, 2020

ittaiz Jan 27, 2020 •

edited

Loading

andyscott Jan 27, 2020

ittaiz left a comment

ittaiz Jan 27, 2020

andyscott Jan 27, 2020

ittaiz Jan 27, 2020

ittaiz Jan 27, 2020

andyscott commented Jan 27, 2020

andyscott commented Jan 28, 2020

ittaiz commented Jan 28, 2020

andyscott commented Jan 28, 2020 via email

ittaiz commented Jan 28, 2020

andyscott commented Jan 29, 2020

smparkes commented Jan 30, 2020 via email •

edited

Loading

ittaiz commented Jan 30, 2020

smparkes commented Jan 30, 2020

unify all of the default info phases implementations #958

unify all of the default info phases implementations #958

Conversation

andyscott commented Jan 27, 2020

Description

Motivation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ittaiz Jan 27, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ittaiz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andyscott commented Jan 27, 2020

andyscott commented Jan 28, 2020

ittaiz commented Jan 28, 2020

andyscott commented Jan 28, 2020 via email

ittaiz commented Jan 28, 2020

andyscott commented Jan 29, 2020

smparkes commented Jan 30, 2020 via email • edited Loading

ittaiz commented Jan 30, 2020

smparkes commented Jan 30, 2020

ittaiz Jan 27, 2020 •

edited

Loading

smparkes commented Jan 30, 2020 via email •

edited

Loading