Remove 1.5.0.dev0 deprecations #5363

stuhood · 2018-01-20T10:14:31Z

Their time has come.

stuhood · 2018-01-20T10:14:47Z

Individual commits should be useful to review independently.

kwlzn

thanks Stu!

kwlzn · 2018-01-22T17:09:58Z

src/python/pants/backend/jvm/targets/jvm_target.py

-      # eagerly evaluate all the globs, which would be a performance drag for goals that
-      # otherwise wouldn't do that (like `list`).
-      for spec in Java.global_instance().injectables_specs_for_key('plugin'):
-        yield spec


would've expected this bit to stay?

The option it was fed by was deprecated and moved elsewhere.

kwlzn · 2018-01-22T17:55:04Z

src/python/pants/bin/goal_runner.py

-                                              build_ignore_patterns,
-                                              exclude_target_regexps,
-                                              subproject_build_roots)
-      return MutableBuildGraph(address_mapper), address_mapper, None, spec_roots


🎆 🍾 🎉 👍

Yay!... note that I don't actually delete MutableBuildGraph, as it is still used in BaseTest (#4867). Which is... even scarier now.

kwlzn · 2018-01-22T18:19:51Z

src/python/pants/build_graph/target.py

-  @property
-  @deprecated('1.5.0.dev0', 'Use `Target.compute_dependency_specs()` instead.')
-  def traversable_dependency_specs(self):
-    """


🤘 🤘 🤘

kwlzn · 2018-01-22T18:32:08Z

src/python/pants/task/task.py

-        'one or more explicit target specs (e.g. `./pants {0} ::`).'.format(goal_name))
-    if not self.context.target_roots and not self.get_options().enable_v2_engine:
-      # For the v1 path, continue the behavior of e.g. `./pants list` implies `./pants list ::`.
-      return self.context.scan().targets(predicate=predicate)


benjyw

Such an awesome change! I'm particularly happy to see the old python backend go away. My only comments (apart from gasps of approval) are around removing comments that reference the past. Thanks for doing this.

benjyw · 2018-01-22T18:42:02Z

src/python/pants/backend/python/BUILD

@@ -133,6 +133,15 @@ python_library(
  ]
 )

+python_library(
+  name = 'tasks2',
+  sources = ['tasks2.py'],


Neat way to deprecate all imports from the former tasks2 package in one go!

benjyw · 2018-01-22T18:44:34Z

src/python/pants/backend/jvm/targets/jvm_target.py

@@ -36,7 +35,6 @@ def __init__(self,
               sources=None,
               provides=None,
               excludes=None,
-               resources=None,


This makes me so happy! That bit of cruft was a thorn in our side for so long.

benjyw · 2018-01-22T18:47:08Z

src/python/pants/backend/python/subsystems/python_setup.py

-             removal_version='1.5.0.dev0', removal_hint='Use --interpreter-constraints instead.',
-             help='The interpreter requirement string for this python environment.')
-    # Note: This will replace two options:
+    # Note: This replaces two options:
    # A) The global --interpreter option in the old python tasks.
    #    That flag is only relevant in the python backend, and should never have been
    #    global to begin with.
    # B) The --interpreter-requirement option above.  That flag merely served to set the


This comment makes no sense now, since the options it mentions are now gone. Why did you choose to keep it?

Just due to a lack of context as to whether leaving some of the history here might be useful to someone debugging something here.

benjyw · 2018-01-22T18:48:21Z

src/python/pants/option/global_options.py

@@ -232,13 +232,6 @@ def register_options(cls, register):
    # TODO: After moving to the new options system these abstraction leaks can go away.
    register('-k', '--kill-nailguns', advanced=True, type=bool,
             help='Kill nailguns before exiting')
-    register('-i', '--interpreter', advanced=True, default=[], type=list,


Again, this fills me with great joy.

benjyw · 2018-01-22T18:49:40Z

tests/python/pants_test/engine/legacy/test_bundle_integration.py

-          'default recursive inclusion of files in directory',
-          'A non-recursive/literal glob should no longer include child paths.'
-      )
+      # NB: The behaviour of this test changed as scheduled in 1.5.0.dev0, because the


Why leave this comment? It seems weird to reference the past. The reader won't know (and hopefully shouldn't care) what this refers to.

My goal here would be that if someone opens a bug against the 1.5.0 dev series indicating that something is broken, someone would find this and determine (more easily) that the change was intentional... I know that these types of historical comments should eventually be deleted, but in the near term they seem helpful.

Dunno, I feel like that's what git history is for. Let's at least put a "deprecation message" with a version on the comments, so that they get deleted in the find-and-replace sweep for a future version. And let's make sure they're reworded in past tense, so the reader doesn't go looking for options that no longer exist.

stuhood · 2018-01-22T21:18:55Z

Only the OSX shard is outstanding. Merging.

benjyw

Just noticed a minor issue...

benjyw · 2018-01-23T02:09:59Z

tests/python/pants_test/backend/python/tasks/BUILD

-  ]
-)
-
-python_tests(


So it looks like the python isort tests were nuked when they should have remained (they had stayed in the old tasks dir instead of being in tasks2, because they weren't affected by the new pipeline changes).

benjyw · 2018-01-30T23:00:26Z

src/python/pants/backend/python/tasks2.py

+from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
+                        unicode_literals, with_statement)
+
+from pants.backend.python.tasks import (GatherSources, PytestPrep, PytestRun, PythonBinaryCreate,


I'm noticing now that this is broken:

A) These imports are all invalid, as those classes are defined in modules inside tasks, not in tasks itself.

B) Even if we fix that, any existing import statements that try to import from modules in tasks2 will now fail, instead of showing the deprecation message but succeeding.

I see no alternative but to restore the modules in tasks2 and have each one just import from the sibling module in tasks and show a deprecation message.

Or am I missing something? This definitely caused pants to fail when I tried to register a tasks2 task.

...hm, of course. sheesh... not sure how I missed that.

I'll tackle that today.

### Problem The `tasks2` deprecations from #5363 did not each have their own modules, and thus were not importable ([see](https://github.com/pantsbuild/pants/pull/5363/files#r164906101)). ### Solution Move the deprecations into their own modules/files.

This is unneeded since pantsbuild#5363 removed the deprecated behavior.

stuhood requested review from benjyw, wisechengyi, mateor and kwlzn January 20, 2018 10:14

stuhood force-pushed the stuhood/1.5.0.dev0-deprecations branch 2 times, most recently from 91ca298 to ad9f559 Compare January 21, 2018 06:33

stuhood added 11 commits January 21, 2018 19:27

Deprecated options scopes.

4d3185d

Remove the old python backend, and deprecate the tasks2 module.

f361882

Remove deprecated engine options.

7c8799d

Remove codegen related deprecations.

72bba99

Remove deprecated target properties.

7a865f7

Remove deprecated compiler plugin locations.

2fe04f5

Remove deprecated resources arguments.

8de8fd7

Remove assorted deprecations.

b983713

Remove deprecated python interpreter options.

55b67b6

Remove default-recursive-bundle deprecation.

ba820bb

Test and dependencies fixes.

cde01ed

stuhood force-pushed the stuhood/1.5.0.dev0-deprecations branch from ad9f559 to cde01ed Compare January 22, 2018 03:33

stuhood mentioned this pull request Jan 22, 2018

pytest test skipped to expedite landing 1.5.0.dev0 deprecations #5369

Closed

Test and import failures.

6f20d64

kwlzn approved these changes Jan 22, 2018

View reviewed changes

benjyw approved these changes Jan 22, 2018

View reviewed changes

wisechengyi approved these changes Jan 22, 2018

View reviewed changes

stuhood merged commit 4aae5dc into pantsbuild:master Jan 22, 2018

stuhood deleted the stuhood/1.5.0.dev0-deprecations branch January 22, 2018 21:19

benjyw reviewed Jan 23, 2018

View reviewed changes

wisechengyi mentioned this pull request Jan 23, 2018

Add back isort tests #5380

Merged

benjyw reviewed Jan 30, 2018

View reviewed changes

stuhood mentioned this pull request Feb 2, 2018

Fix tasks2 deprecations #5421

Merged

illicitonion added a commit to twitter/pants that referenced this pull request Jun 19, 2018

Remove option to include directory in files list

a613448

This is unneeded since pantsbuild#5363 removed the deprecated behavior.

illicitonion mentioned this pull request Jun 19, 2018

Remove option to include directory in files list #5986

Closed

chysi mentioned this pull request Sep 3, 2019

Python coverage cannot exclude lines #8234

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove 1.5.0.dev0 deprecations #5363

Remove 1.5.0.dev0 deprecations #5363

stuhood commented Jan 20, 2018

stuhood commented Jan 20, 2018

kwlzn left a comment

kwlzn Jan 22, 2018

stuhood Jan 22, 2018

kwlzn Jan 22, 2018

stuhood Jan 22, 2018 •

edited

Loading

kwlzn Jan 22, 2018

kwlzn Jan 22, 2018

benjyw left a comment

benjyw Jan 22, 2018

benjyw Jan 22, 2018

benjyw Jan 22, 2018

stuhood Jan 22, 2018

benjyw Jan 22, 2018

benjyw Jan 22, 2018

stuhood Jan 22, 2018

benjyw Jan 22, 2018

stuhood commented Jan 22, 2018

benjyw left a comment

benjyw Jan 23, 2018

benjyw Jan 30, 2018

stuhood Feb 1, 2018

stuhood Feb 1, 2018

stuhood Feb 2, 2018

Remove 1.5.0.dev0 deprecations #5363

Remove 1.5.0.dev0 deprecations #5363

Conversation

stuhood commented Jan 20, 2018

stuhood commented Jan 20, 2018

kwlzn 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

stuhood Jan 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benjyw 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

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

stuhood commented Jan 22, 2018

benjyw 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

Choose a reason for hiding this comment

stuhood Jan 22, 2018 •

edited

Loading