recipe_modules/shutil/api.py - Issue 1866313002: recipe engine: add tempfile and shutil modules

Side by Side Diff: recipe_modules/shutil/api.py

Issue 1866313002: recipe engine: add tempfile and shutil modules (Closed) Base URL: https://github.com/luci/recipes-py.git@master

Patch Set: presubmit Created 4 years, 8 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
(Empty)
	1 # Copyright 2016 The Chromium Authors. All rights reserved.

	2 # Use of this source code is governed by a BSD-style license that can be

	3 # found in the LICENSE file.

	4

	5 from recipe_engine import recipe_api

	6

	7 class ShutilApi(recipe_api.RecipeApi):
	martiniss 2016/04/07 17:35:22 I'm uncomfortable having this, when we have the fi I'm uncomfortable having this, when we have the file recipe module at https://chromium.googlesource.com/chromium/tools/build.git/+/master/scripts/s... It would be better to just upstream this, since most of those functions aren't build dependent. However, several of them depend on depot_tools/build python scripts to function properly, so they can't be put into the recipe engine. I would prefer to just inline this wherever you need this for now; I know tandrii did for his recipe at https://chromium.googlesource.com/chromium/tools/build.git/+/master/scripts/s... Paweł Hajdan Jr. 2016/04/07 20:39:04 With shutil module here, we can move build-indepen Show quoted text On 2016/04/07 at 17:35:22, martiniss wrote: > I'm uncomfortable having this, when we have the file recipe module at https://chromium.googlesource.com/chromium/tools/build.git/+/master/scripts/s... With shutil module here, we can move build-independent methods from file module here. Note however that file.rmtree currently depends on chromium_utils, and I'm not sure if we can change that without breakage. Show quoted text > It would be better to just upstream this, since most of those functions aren't build dependent. However, several of them depend on depot_tools/build python scripts to function properly, so they can't be put into the recipe engine. > > I would prefer to just inline this wherever you need this for now; I know tandrii did for his recipe at https://chromium.googlesource.com/chromium/tools/build.git/+/master/scripts/s... I'm leaning towards providing shutil here - at least there'd be one place, rather than several different copies sprinkled throughout codebase. Is yours a strong opinion? If so, could you provide rationale for it? FWIW mine is like above - less number of places affected by ugliness, if there is any ugliness at all. martiniss 2016/04/07 21:34:02 If you're volunteering, please add a TODO :) I was Show quoted text On 2016/04/07 at 20:39:04, Paweł Hajdan Jr. wrote: > On 2016/04/07 at 17:35:22, martiniss wrote: > > I'm uncomfortable having this, when we have the file recipe module at https://chromium.googlesource.com/chromium/tools/build.git/+/master/scripts/s... > > With shutil module here, we can move build-independent methods from file module here. If you're volunteering, please add a TODO :) I was planning on doing that eventually, once the recipe roller and tryjobs have stabilized more. Show quoted text > > Note however that file.rmtree currently depends on chromium_utils, and I'm not sure if we can change that without breakage. > > > It would be better to just upstream this, since most of those functions aren't build dependent. However, several of them depend on depot_tools/build python scripts to function properly, so they can't be put into the recipe engine. > > > > I would prefer to just inline this wherever you need this for now; I know tandrii did for his recipe at https://chromium.googlesource.com/chromium/tools/build.git/+/master/scripts/s... > > I'm leaning towards providing shutil here - at least there'd be one place, rather than several different copies sprinkled throughout codebase. Ok, can you change the inline uses of it to use this module then, in a separate CL in build then? Show quoted text > > Is yours a strong opinion? If so, could you provide rationale for it? FWIW mine is like above - less number of places affected by ugliness, if there is any ugliness at all. Paweł Hajdan Jr. 2016/04/08 07:46:15 Added TODO and I plan to address it soon. My recip Show quoted text On 2016/04/07 at 21:34:02, martiniss wrote: > On 2016/04/07 at 20:39:04, Paweł Hajdan Jr. wrote: > > On 2016/04/07 at 17:35:22, martiniss wrote: > > > I'm uncomfortable having this, when we have the file recipe module at https://chromium.googlesource.com/chromium/tools/build.git/+/master/scripts/s... > > > > With shutil module here, we can move build-independent methods from file module here. > > If you're volunteering, please add a TODO :) I was planning on doing that eventually, once the recipe roller and tryjobs have stabilized more. Added TODO and I plan to address it soon. My recipe roller work depends on this CL landing.
	8 def rmtree(self, path, **kwargs):

	9 self.m.python.inline(

	10 'rmtree %s' % path,

	11 """

	12 import shutil, sys

	13 shutil.rmtree(sys.argv[1])

	14 """,

	15 args=[path],

	16 **kwargs)

OLD	NEW

« no previous file with comments | « recipe_modules/shutil/__init__.py ('k') | recipe_modules/shutil/example.py » ('j') | recipe_modules/tempfile/api.py » ('J')