-
-
Notifications
You must be signed in to change notification settings - Fork 652
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
Expose rules from language backends with an application to python_dist() creation #5747
Merged
wisechengyi
merged 24 commits into
pantsbuild:master
from
cosmicexplorer:expose-rules-from-language-backends
Apr 28, 2018
Merged
Changes from all commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
8fd04c2
add some simple examples to demonstrate how to use backend rules
cosmicexplorer 258c767
...actually make the changes to consume backend rules in register.py
cosmicexplorer c3dc339
expand a couple docstrings
cosmicexplorer 2b7c33d
revert accidental change to a test target
cosmicexplorer 5620ce6
remove extraneous log statement
cosmicexplorer d421e5a
fix lint errors
cosmicexplorer 9ac7e0f
respond to review comments
cosmicexplorer 8f5ee6a
add unit test and change field name
cosmicexplorer 5d847a4
fix import order
cosmicexplorer efe75b8
respond to review comments
cosmicexplorer c0b4b36
add native backend to release.sh
cosmicexplorer 6a86987
isolate native toolchain path and hope for the best
cosmicexplorer a39f21d
add backend plugin dependency (intentionally named differently)
cosmicexplorer 26cc5dd
explicitly set CC and CXX because distutils
cosmicexplorer 4ff80a2
add LDSHARED to setup.py environment for fun
cosmicexplorer 1c5218b
try using gcc on travis
cosmicexplorer 94f68f5
fix datatype() declarations
cosmicexplorer 1c7e08d
fix old datatype usages
cosmicexplorer 152e322
revert whitespace edit
cosmicexplorer 514c1de
unclear why these are necessary
cosmicexplorer 1cfe416
replicate the previous behavior
cosmicexplorer 32b39e3
try again to isolate the path, this time using ARCHFLAGS
cosmicexplorer 14eeaff
fix arguments for edited setup py env rule
cosmicexplorer 7236837
restore the previous functionality again
cosmicexplorer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
# coding=utf-8 | ||
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md). | ||
# Licensed under the Apache License, Version 2.0 (see LICENSE). | ||
|
||
python_library( | ||
dependencies=[ | ||
'src/python/pants/backend/native/subsystems', | ||
'src/python/pants/engine:rules', | ||
'src/python/pants/util:objects', | ||
'src/python/pants/util:osutil', | ||
], | ||
) |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
# coding=utf-8 | ||
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md). | ||
# Licensed under the Apache License, Version 2.0 (see LICENSE). | ||
|
||
from __future__ import (absolute_import, division, generators, nested_scopes, print_function, | ||
unicode_literals, with_statement) | ||
|
||
from pants.backend.native.subsystems.native_toolchain import NativeToolchain | ||
from pants.engine.rules import RootRule, SingletonRule | ||
from pants.util.objects import datatype | ||
from pants.util.osutil import get_normalized_os_name | ||
|
||
|
||
class Platform(datatype(['normalized_os_name'])): | ||
|
||
def __new__(cls): | ||
return super(Platform, cls).__new__(cls, get_normalized_os_name()) | ||
|
||
|
||
def rules(): | ||
return [ | ||
RootRule(NativeToolchain), | ||
SingletonRule(Platform, Platform()), | ||
] |
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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to see some unit tests for this in
test_extension_loader.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's only 3 lines long... seems like overkill?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair. My thought was that as this is a plugin-writer facing change, it should have tests. If we're currently thinking of it as being experimental, then it's fine without. It'd be nice if that were noted somehow though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's definitely covered by tests, in that the rest of this change relies on it working... it just doesn't seem worth a unit test. It's definitely not code with high cyclomatic complexity, for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I totally agree that it's important to test anything that goes out to plugin writers, but as far as I am aware this change only affects backends within the pants codebase. I did however write an extremely small test anyway (see here). The natural extension of this PR is to do the same for plugins, and I absolutely think that change be paired with testprojects using it and integration tests as well, probably, but for this PR I think passing the python dist testing that exists is sufficient to prove it works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!