Mateo.1.4.0+fs x fork #2
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This avoids needing to compile the whole rust engine and stuff, when just bundling up a pex.
This helps pick up recent runtime changes (PEX_PYTHON_PATH et al) for pants as built as a pex.
…sses. (pantsbuild#5508) Problem Currently, env vars set in post-pantsd-launching runs that use the daemon fail to inherit env vars correctly in child processes of pantsd-runners. Anything that relies on this will be broken with the daemon enabled for ./pants run, ./pants test, etc. Additionally, env vars that are set during the pantsd-launching run will currently leak stale env state into child processes of pantsd-runners. Solution Set the remote environment as the local environment for DaemonPantsRunner from a clean state. Result Tests pass.
This allows the test writer to ensure their log statements fire when expected in the face of non-local manipulation of the global python logging subsystem. Fixes pantsbuild#5417
Similar to how Wire generates Android-optimized Java code from protobufs, [Microsoft Thrifty](https://github.com/Microsoft/thrifty) generates Android-optimized Java code from thrift IDL files. Here we add a new `java_thrifty_library` target type to integrate Thrifty with Pants.
The recently added `java_thrifty_library` does not have `build_file_aliases` set, which makes the new target type inaccessible in build files. This requirement is already documented in https://github.com/pantsbuild/pants/blob/master/contrib/README.md so here we just fix the bug, vs. update the docs too.
this is good for a ~40x speedup in my test case: before: [omerta pants (kwlzn/faster_v2_changed)]$ time ./pants --changed-parent=HEAD~60 list > a real 4m26.282s user 2m59.511s sys 1m25.936s after: [omerta pants (kwlzn/faster_v2_changed)]$ time ./pants --changed-parent=HEAD~60 list > b real 0m7.668s user 0m6.251s sys 0m1.425s Fixes pantsbuild#5570
…m for old engine. bump version.
This fails the javadoc generation because of API changes, obvs. The full fix is probably to change this to checking only for java_library. Using this hammer today.
This engine ageing out of the upstream storage (or whatever) is a huge pain, we need to migrate this s3 cache and remaining small fixes back to upstream
This is a temporary rollback to this version, our next Pants upgrade will make the call to either grandfather the relevant violations into our global exlusions, fix those violations, or remove our internal pep8/pylint tasks in favor of upstream python_checks contrib module, which respects '# noqa' for all pep8 violations, unlike the pep8 library itself which only allows annotation opt-out for certain classes of violation
This (or a range) should be pushed upstream
This is a simple and dirty hack - even figuring out what this code was doing was incredibly complicated - not a very fun library. I tried to carry it forward into something we could land upstream, we are going to have to do that sooner or later, this is an expensice thing to fork long term. Here is where that ended up: https://github.com/mateor/pants/commits/mateo.go_buildgen I think it works fully, but it uses alternate_target_roots to force the targets in context and that breaks a ton of the assumptions in the tests. so that will be a pain, but maybe a pain we have to slog through.
* Memoize org.scalatest.Suite class loading * Load Suite class in static initialization * No final with try catch * Also load junit runner class in static init * Fix small style issues * Touch file in attempt to get CI to work * Revert "Touch file in attempt to get CI to work" This reverts commit c69dfc7. * Edit the line that CI is complaining about ¯\_(ツ)_/¯ * Another edit to be safe * Revert "Another edit to be safe" This reverts commit 694724e. * Revert "Edit the line that CI is complaining about ¯\_(ツ)_/¯" This reverts commit 7ec7423. * Disable fmt.google-java-format
…pace
I couldn't figure out anything simple here. I would have preferred to add
a classifier or something but I couldn't find anything obvious.
The junit-runner dependencies are also not published at their current
SemVer, so the transitive deps fail when publishing the runner alone.
At the end of the day, I munged the namespace of everything in the
junit-runner's closure and uploaded to our internal jar server.
All of the *fs jars are only available internally.
The patch we are backporting is already on master but we may
have already released the 1.4.0 final, which means we are stuck
with the forked jars until we consume 1.5.0, which is the biggest
breaking update since the 1.2.0 upgrade.
So:
1. ./pants publish --publish-jar-local=~/.m3 --publish-jar-force --no-publish-jar-dryrun --publish-jar-override=org.pantsbuild#junit-runner-fs=1.0.24 --publish-jar-override=org.pantsbuild#junit-runner-withretry-fs=0.0.17 --publish-jar-override=org.pantsbuild#junit-runner-annotations-fs=0.0.21 --publish-jar-override=org.pantsbuild#args4j-fs=0.0.19 --no-publish-jar-local-snapshot src/java/org/pantsbuild::
2. Upload the jars manually to Nexus.
- This might have been easier to port the publish xml from our
internal repo, but I didn't want to risk accidentally pushing to
the upstream Pants maven account.
The upload process:
- Go to Nexus
- Log in
- Repositories tab
- 3rdparty Releases
- Upload Artifact tab in lower pane
- Pom
- GAV Definition: From Pom
- Select pom from disk
- DONT UPLOAD YET
- Jars
- Select Jars to upload - 3x required:
- jar, sources.jar, javadoc.jar
- Just let autofill, ignore weirdness
- Add Artifacts for all 3 in a row
- NOW click upload
2. I also ported the foursquare/ivysettings.xml chain repos so that Pants
could test the resolution and run unit tests. If you messed up any
of the steps above you may see ClassNotFound and you should click
around the 3rdparty Releases repository.
Be aggressive with local cleans - you could really confuse yourself!
jonshea
pushed a commit
that referenced
this pull request
Nov 11, 2020
Per pantsbuild#7649, we are working to remote unit tests in CI. This implements step #2, which allows us to run unit tests locally (several, but not all tests). Here, we set up `pants.remote.ini` so that users only must point to it and provide the authentication token.
jonshea
pushed a commit
that referenced
this pull request
Nov 11, 2020
### Problem MyPy does not understand `Goal.Options`, e.g. `List.Options` and `Fmt.Options`, because the implementation relies on runtime creation of the class. MyPy only can understand classes declared at compile time. This is the same reason we needed to rework `pants.util.objects.enum`, `pants.util.objects.datatype`, and `pants.engine.objects.Collection`. At the same time, we want to preserve as much of the original API as possible because it provided a useful coupling between the `Goal` class and its underlying subsystem. We need to preserve features like the goal's docstring populating `./pants help`, for example. One of the challenges is that the original implementation requires the Subsystem inner class to have knowledge of the Goal outer class. This is not a well-supported idiom in Python: https://stackoverflow.com/questions/2024566/access-outer-class-from-inner-class-in-python. ### Attempted solutions #### Attempt #1: pass outer Goal directly to inner Options I started by trying to implement https://stackoverflow.com/a/7034206. The issue is that we make heavy use of `@classmethod` and `@classproperty`, so we need to be able to access `outer_cls` at the `cls` scope and this pattern only allows using it at the instance scope (`self`). #### Attempt #2: `Goal.Options` directly instantiated with reference to `Goal` I then tried this pattern: ```python class List(Goal): name = "list" @classmethod def register_options(cls, register): ... class ListOptions(Options): goal_cls = List @rule def list(..., options: ListOptions) -> List: ... ``` This required rewriting `rules.py` to ensure that the `Options` were properly registered with the rule graph: https://github.com/pantsbuild/pants/blob/b88dc0f7d9c0ce322714b74d78f312b4c4b847a2/src/python/pants/engine/rules.py#L138-L155 This worked fine when a rule explicitly requested the `Options` in its rule params, but it meant that rules without explicitly registered options, like `fmt` and `lint`, were no longer registered with the engine! There was no way to feed in the sibling `FmtOptions` or `LintOptions` because these would have to be explicitly created and somehow passed to the engine, even though the corresponding rules had no need to directly consume those subsystems. ### Solution: new `GoalSubsystem` class Setting up a `Goal` will now involve two steps: 1. Register a `GoalSubsystem` that has the name of the goal, the docstring, and any options (if any). 1. Register a `Goal`, which only needs to point `subsystem_cls` to the sibling `GoalSubsystem`. For example: ```python class ListOptions(GoalSubsystem): """List all the targets.""" name = "list" @classmethod def register_options(cls, register): ... class List(Goal): subsystem_cls = ListOptions ``` Rules may then explicitly request the `GoalSubsystem`, e.g. `ListOptions`. Consumers of the `Goal`, like `rules.py` and `engine_initializer.py`, simply access `Goal.subsystem_cls` to get the attributes they previously accessed directly. #### Conceptual motivation This strengthens the concept that Subsystems are the sole mechanism for consuming options in V2. The `GoalSubsystem` may be seen as the external API—containing all the options, the goal name, and the goal description seen by the outside world. Meanwhile, the `Goal` consumes that subsystem (just like the `black.fmt` rule consumes the `Black` subsystem) to produce a new type used internally by the engine. ### Result MyPy now understands V2 goals, which unblocks us from using MyPy with V2 code and tests (`TestBase` and `PantsRunIntegration` are blocked by this issue).
jonshea
pushed a commit
that referenced
this pull request
Nov 11, 2020
…uild#9466) Twice now, we've had `Sources` fields that require a specific number of files. So, this PR formalizes a mechanism to do this. Error message #1: > ERROR: The 'sources' field in target build-support/bin:check_banned_imports must have 3 files, but it had 1 file. Error message #2: > ERROR: The 'sources' field in target build-support/bin:check_banned_imports must have 2 or 3 files, but it had 1 file. Error message #3: > ERROR: The 'sources' field in target build-support/bin:check_banned_imports must have a number of files in the range `range(20, 20000, 10)`, but it had 1 file. [ci skip-rust-tests] # No Rust changes made. [ci skip-jvm-tests] # No JVM changes made.
jonshea
pushed a commit
that referenced
this pull request
Nov 11, 2020
## Goals of design See https://docs.google.com/document/d/1tJ1SL3URSXUWlrN-GJ1fA1M4jm8zqcaodBWghBWrRWM/edit?ts=5ea310fd for more info. tl;dr: 1) Protocols now only have one generic target, like `avro_library`. This means that call sites must declare which language should be generated from that protocol. * Must be declarative. 2) You can still get the original protocol sources, e.g. for `./pants filedeps`. 3) Must work with subclassing of fields. 4) Must be extensible. * Example: Pants only implements Thrift -> Python. A plugin author should be able to add Thrift -> Java. ## Implementation Normally, to hydrate sources, we call `await Get[HydratedSources](HydrateSourcesRequest(my_sources_field))`. We always use the exact same rule to do this because all `sources` fields are hydrated identically. Here, each codegen rule is unique. So, we need to use unions. This means that we also need a uniform product for each codegen rule for the union to work properly. This leads to: ```python await Get[GeneratedSources](GenerateSourcesRequest, GeneratePythonFromAvroRequest(..)) await Get[GeneratedSources](GenerateSourcesRequest, GenerateJavaFromThriftRequest(..)) ``` Each `GenerateSourcesRequest` subclass gets registered as a union rule. This achieves goal #4 of extensibility. -- To still work with subclassing of fields (goal #3), each `GenerateSourcesRequest` declares the input type and output type, which then allows us to use `isinstance()` to accommodate subclasses: ```python class GenerateFortranFromAvroRequest(GenerateSourcesRequest): input = AvroSources output = FortranSources ``` -- To achieve goals #1 and #2 of allowing call sites to declaratively either get the original protocol sources or generated sources, we hook up codegen to the `hydrate_sources` rule and `HydrateSourcesRequest` type: ```python protocol_sources = await Get[HydratedSources](HydrateSourcesRequest(avro_sources, for_sources_types=[FortranSources], codegen_enabled=True)) ``` [ci skip-rust-tests] [ci skip-jvm-tests]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
(explain the context of the problem and why you're making this change. include
references to all relevant github issues.)
Solution
(describe the modifications you've made.)
Result
(describe how your changes affect the end-user behavior of the system. this section is
optional, and should generally be summarized in the title of the pull request.)