Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.
On July 24, 2015, 10:57 a.m., Brian Wickman wrote: src/main/python/apache/thermos/observer/BUILD, line 74 https://reviews.apache.org/r/36700/diff/2/?file=1020365#file1020365line74 This puts an aurora dependency on thermos and I imagine causes a cycle in the build graph (pants may just silently ignore it.) Let's just remove the thermos_observer entry point from this package entirely by deleting the .with_binaries clause. Bill Farner wrote: This target (`src/main/python/apache/thermos/observer`) is a top-level target, only used by `build-support/release/make-python-sdists`, so i think we're safe from a cycle. I'm not strictly against removing the `with_binaries`, but i assume it is there for a reason and something would break? Or do we just not use the sdists [this way]? Brian Wickman wrote: Afaik nobody uses these in practice. They're only useful for when you install the sdists via pip or pex directly. Instead we always build pex binaries using pants which delegates to the python_binary target. Kevin Sweeney wrote: I answered someone's question in IRC with a gist ~2yrs ago https://gist.github.com/kevints/8361414 so it does seem likely these are used in practice. Okay with removing them but a NEWS entry would be good form. Bill Farner wrote: Should i just kill the sdists entirely? Brian Wickman wrote: I'd prefer to go in the opposite direction and kill pants. Bill Farner wrote: I'd like to find a path to land a fix as packaging is currently broken/blocked. I believe the current diff is the closest thing to compromise between the various opinions around this change. Please let me know what you think. -1 as this fix breaks the observer entry point entirely and produces an sdist that is invalid ``` (observer.venv)~aurora git aurora/. 36700 % pip install -f dist apache.thermos.observer --pre You are using pip version 6.1.1, however version 7.1.0 is available. You should consider upgrading via the 'pip install --upgrade pip' command. Collecting apache.thermos.observer Could not find a version that satisfies the requirement apache.thermos.observer (from versions: 0.10.0-SNAPSHOT) No matching distribution found for apache.thermos.observer ``` so pants writes this sdist with a dependency on itself since it's already broken I'd suggest removing the entry point entirely (so we're not advertising a feature we don't actually have). - Kevin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36700/#review92944 --- On July 23, 2015, 5 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36700/ --- (Updated July 23, 2015, 5 p.m.) Review request for Aurora, Kevin Sweeney and Brian Wickman. Bugs: AURORA-1381 https://issues.apache.org/jira/browse/AURORA-1381 Repository: aurora Description --- The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging). This removes targets that Aurora users should not be consuming. Diffs - build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e src/main/python/apache/thermos/cli/bin/thermos.py fcef38d33e7ca2005f35c3bdb90ffee6aeade3af src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 src/main/python/apache/thermos/observer/bin/thermos_observer.py 39d3994a6163746e853cd21fc4c3585edc2b54cb Diff: https://reviews.apache.org/r/36700/diff/ Testing --- ./build-support/jenkins/build.sh ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh (as far as it currently gets on master, anyhow, due to AURORA-1409) Thanks, Bill Farner
Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.
Me too, but I'm okay with killing the sdist targets as an intermediate step and reintroducing them with a non-pants build On Friday, July 24, 2015, Brian Wickman wick...@apache.org wrote: On July 24, 2015, 5:57 p.m., Brian Wickman wrote: src/main/python/apache/thermos/observer/BUILD, line 74 https://reviews.apache.org/r/36700/diff/2/?file=1020365#file1020365line74 This puts an aurora dependency on thermos and I imagine causes a cycle in the build graph (pants may just silently ignore it.) Let's just remove the thermos_observer entry point from this package entirely by deleting the .with_binaries clause. Bill Farner wrote: This target (`src/main/python/apache/thermos/observer`) is a top-level target, only used by `build-support/release/make-python-sdists`, so i think we're safe from a cycle. I'm not strictly against removing the `with_binaries`, but i assume it is there for a reason and something would break? Or do we just not use the sdists [this way]? Brian Wickman wrote: Afaik nobody uses these in practice. They're only useful for when you install the sdists via pip or pex directly. Instead we always build pex binaries using pants which delegates to the python_binary target. Kevin Sweeney wrote: I answered someone's question in IRC with a gist ~2yrs ago https://gist.github.com/kevints/8361414 so it does seem likely these are used in practice. Okay with removing them but a NEWS entry would be good form. Bill Farner wrote: Should i just kill the sdists entirely? I'd prefer to go in the opposite direction and kill pants. - Brian --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36700/#review92944 --- On July 24, 2015, midnight, Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36700/ --- (Updated July 24, 2015, midnight) Review request for Aurora, Kevin Sweeney and Brian Wickman. Bugs: AURORA-1381 https://issues.apache.org/jira/browse/AURORA-1381 Repository: aurora Description --- The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging). This removes targets that Aurora users should not be consuming. Diffs - build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e src/main/python/apache/thermos/cli/bin/thermos.py fcef38d33e7ca2005f35c3bdb90ffee6aeade3af src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 src/main/python/apache/thermos/observer/bin/thermos_observer.py 39d3994a6163746e853cd21fc4c3585edc2b54cb Diff: https://reviews.apache.org/r/36700/diff/ Testing --- ./build-support/jenkins/build.sh ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh (as far as it currently gets on master, anyhow, due to AURORA-1409) Thanks, Bill Farner -- Sent from Gmail Mobile
Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36700/#review92944 --- src/main/python/apache/thermos/observer/BUILD (line 74) https://reviews.apache.org/r/36700/#comment147203 This puts an aurora dependency on thermos and I imagine causes a cycle in the build graph (pants may just silently ignore it.) Let's just remove the thermos_observer entry point from this package entirely by deleting the .with_binaries clause. - Brian Wickman On July 24, 2015, midnight, Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36700/ --- (Updated July 24, 2015, midnight) Review request for Aurora, Kevin Sweeney and Brian Wickman. Bugs: AURORA-1381 https://issues.apache.org/jira/browse/AURORA-1381 Repository: aurora Description --- The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging). This removes targets that Aurora users should not be consuming. Diffs - build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e src/main/python/apache/thermos/cli/bin/thermos.py fcef38d33e7ca2005f35c3bdb90ffee6aeade3af src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 src/main/python/apache/thermos/observer/bin/thermos_observer.py 39d3994a6163746e853cd21fc4c3585edc2b54cb Diff: https://reviews.apache.org/r/36700/diff/ Testing --- ./build-support/jenkins/build.sh ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh (as far as it currently gets on master, anyhow, due to AURORA-1409) Thanks, Bill Farner
Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.
On July 24, 2015, 5:57 p.m., Brian Wickman wrote: src/main/python/apache/thermos/observer/BUILD, line 74 https://reviews.apache.org/r/36700/diff/2/?file=1020365#file1020365line74 This puts an aurora dependency on thermos and I imagine causes a cycle in the build graph (pants may just silently ignore it.) Let's just remove the thermos_observer entry point from this package entirely by deleting the .with_binaries clause. Bill Farner wrote: This target (`src/main/python/apache/thermos/observer`) is a top-level target, only used by `build-support/release/make-python-sdists`, so i think we're safe from a cycle. I'm not strictly against removing the `with_binaries`, but i assume it is there for a reason and something would break? Or do we just not use the sdists [this way]? Brian Wickman wrote: Afaik nobody uses these in practice. They're only useful for when you install the sdists via pip or pex directly. Instead we always build pex binaries using pants which delegates to the python_binary target. Kevin Sweeney wrote: I answered someone's question in IRC with a gist ~2yrs ago https://gist.github.com/kevints/8361414 so it does seem likely these are used in practice. Okay with removing them but a NEWS entry would be good form. Bill Farner wrote: Should i just kill the sdists entirely? I'd prefer to go in the opposite direction and kill pants. - Brian --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36700/#review92944 --- On July 24, 2015, midnight, Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36700/ --- (Updated July 24, 2015, midnight) Review request for Aurora, Kevin Sweeney and Brian Wickman. Bugs: AURORA-1381 https://issues.apache.org/jira/browse/AURORA-1381 Repository: aurora Description --- The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging). This removes targets that Aurora users should not be consuming. Diffs - build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e src/main/python/apache/thermos/cli/bin/thermos.py fcef38d33e7ca2005f35c3bdb90ffee6aeade3af src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 src/main/python/apache/thermos/observer/bin/thermos_observer.py 39d3994a6163746e853cd21fc4c3585edc2b54cb Diff: https://reviews.apache.org/r/36700/diff/ Testing --- ./build-support/jenkins/build.sh ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh (as far as it currently gets on master, anyhow, due to AURORA-1409) Thanks, Bill Farner
Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36700/#review92845 --- Master (24088ef) is green with this patch. ./build-support/jenkins/build.sh However, it appears that it might lack test coverage. I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On July 24, 2015, midnight, Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36700/ --- (Updated July 24, 2015, midnight) Review request for Aurora, Kevin Sweeney and Brian Wickman. Bugs: AURORA-1381 https://issues.apache.org/jira/browse/AURORA-1381 Repository: aurora Description --- The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging). This removes targets that Aurora users should not be consuming. Diffs - build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e src/main/python/apache/thermos/cli/bin/thermos.py fcef38d33e7ca2005f35c3bdb90ffee6aeade3af src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 src/main/python/apache/thermos/observer/bin/thermos_observer.py 39d3994a6163746e853cd21fc4c3585edc2b54cb Diff: https://reviews.apache.org/r/36700/diff/ Testing --- ./build-support/jenkins/build.sh ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh (as far as it currently gets on master, anyhow, due to AURORA-1409) Thanks, Bill Farner
Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.
On July 22, 2015, 7:56 p.m., Kevin Sweeney wrote: build-support/packaging/debian/rules, line 37 https://reviews.apache.org/r/36700/diff/1/?file=1018921#file1018921line37 Can you delete the code these targets reference as well, as presumably there are now source files without BUILD file owners in our tree Done. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36700/#review92639 --- On July 22, 2015, 6:50 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36700/ --- (Updated July 22, 2015, 6:50 p.m.) Review request for Aurora, Kevin Sweeney and Brian Wickman. Bugs: AURORA-1381 https://issues.apache.org/jira/browse/AURORA-1381 Repository: aurora Description --- The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging). This removes targets that Aurora users should not be consuming. Diffs - build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 Diff: https://reviews.apache.org/r/36700/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36700/#review92633 --- Master (b64f7b6) is green with this patch. ./build-support/jenkins/build.sh However, it appears that it might lack test coverage. I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On July 22, 2015, 6:50 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36700/ --- (Updated July 22, 2015, 6:50 p.m.) Review request for Aurora, Kevin Sweeney and Brian Wickman. Bugs: AURORA-1381 https://issues.apache.org/jira/browse/AURORA-1381 Repository: aurora Description --- The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging). This removes targets that Aurora users should not be consuming. Diffs - build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 Diff: https://reviews.apache.org/r/36700/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36700/#review92639 --- build-support/packaging/debian/rules (line 37) https://reviews.apache.org/r/36700/#comment146861 Can you delete the code these targets reference as well, as presumably there are now source files without BUILD file owners in our tree - Kevin Sweeney On July 22, 2015, 11:50 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36700/ --- (Updated July 22, 2015, 11:50 a.m.) Review request for Aurora, Kevin Sweeney and Brian Wickman. Bugs: AURORA-1381 https://issues.apache.org/jira/browse/AURORA-1381 Repository: aurora Description --- The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging). This removes targets that Aurora users should not be consuming. Diffs - build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 Diff: https://reviews.apache.org/r/36700/diff/ Testing --- Thanks, Bill Farner
