track clobbering in .deps, and skip warning when updating an expected…

…-to-clobber-target. fixes #2

ocaml/gup/builder.ml

@@ -128,7 +128,7 @@ class target (buildscript:Gupfile.buildscript) =
 				log#trace("no build needed");
 				Lwt.return false
 			) else (
-				state#perform_build exe_path (fun exe ->
+				state#perform_build exe_path (fun exe deps ->
 
 					let basedir = buildscript#basedir in
 					Util.makedirs basedir;
@@ -176,16 +176,20 @@ class target (buildscript:Gupfile.buildscript) =
 						in
 						lwt new_mtime = Util.get_mtime self#path in
 						let target_changed = neq (Option.compare ~cmp:Big_int.compare) mtime new_mtime in
-						if target_changed then (
+						lwt () = if target_changed then (
 							let p = Option.print Big_int.print in
 							log#trace "old_mtime=%a, new_mtime=%a" p mtime p new_mtime;
-							if not (Sys.is_directory self#path) then (
+							if (Sys.is_directory self#path) then Lwt.return_unit else (
 								(* directories often need to be created directly *)
-								log#warn "%s modified %s directly"
-									(Util.relpath ~from:Var.root_cwd exe_path)
-									self#path
-							);
-						);
+								let expect_clobber = match deps with None -> false | Some d -> d#clobbers in
+								if (not (update && expect_clobber)) then (
+									log#warn "%s modified %s directly"
+										(Util.relpath ~from:Var.root_cwd exe_path)
+										self#path
+								);
+								state#mark_clobbers
+							)
+						) else Lwt.return_unit in
 
 						match ret with
 							| Unix.WEXITED 0 -> begin

ocaml/gup/logging.ml

@@ -23,7 +23,7 @@ let ord lvl =
 let string_of_level lvl =
 	match lvl with
 	| Error -> "ERROR"
-	| Warn  -> "WARN"
+	| Warn  -> "WARNING"
 	| Info  -> "INFO"
 	| Debug -> "DEBUG"
 	| Trace -> "TRACE"

ocaml/gup/state.ml

@@ -43,6 +43,7 @@ type dependency_type =
 	| Builder
 	| BuildTime
 	| AlwaysRebuild
+	| ClobbersTarget
 
 let log = Logging.get_logger "gup.state"
 let meta_dir_name = ".gup"
@@ -85,6 +86,7 @@ let current_run_id = new run_id Var.run_id
 type 'a intermediate_dependencies = {
 	checksum: string option ref;
 	run_id: run_id option ref;
+	clobbers: bool ref;
 	rules: 'a list ref;
 }
 
@@ -101,6 +103,7 @@ let string_of_dependency_type = function
 	| BuildTime -> "built"
 	| Builder -> "builder"
 	| AlwaysRebuild -> "always"
+	| ClobbersTarget -> "clobbers"
 
 
 let serializable_dependencies = [
@@ -110,6 +113,7 @@ let serializable_dependencies = [
 	{ tag = BuildTime; num_fields = 1; };
 	{ tag = Builder; num_fields = 3; };
 	{ tag = AlwaysRebuild; num_fields = 0; };
+	{ tag = ClobbersTarget; num_fields = 0; };
 ]
 
 let tag_assoc     = serializable_dependencies |> List.map (fun dep -> (dep.tag, dep))
@@ -255,6 +259,10 @@ and dependencies target_path (data:base_dependency intermediate_dependencies) =
 				(Option.print String.print) !(data.checksum)
 				(List.print print_obj) !(data.rules)
 
+		method checksum = !(data.checksum)
+
+		method clobbers = !(data.clobbers)
+
 		method is_dirty (buildscript: Gupfile.buildscript) (built:bool) : (target_state list) dirty_result Lwt.t =
 			if not (Util.lexists target_path) then (
 				log#debug "DIRTY: %s (target does not exist)" target_path;
@@ -292,8 +300,6 @@ and dependencies target_path (data:base_dependency intermediate_dependencies) =
 				in
 				collapse !(data.rules) []
 			)
-
-		method checksum = !(data.checksum)
 end
 
 and dependency_builder target_path (input:Lwt_io.input_channel) = object (self)
@@ -322,8 +328,9 @@ and dependency_builder target_path (input:Lwt_io.input_channel) = object (self)
 				let parse_cs cs = if cs = empty_field then None else Some cs in
 				let parse_mtime mtime = if mtime = empty_field then None else Some (Big_int.of_string mtime) in
 				match (typ.tag, fields) with
-					| (Checksum, [cs]) -> update_singleton rv.checksum (Some cs)
-					| (RunId, [r])     -> update_singleton rv.run_id (Some (new run_id r))
+					| (Checksum, [cs])      -> update_singleton rv.checksum (Some cs)
+					| (RunId, [r])          -> update_singleton rv.run_id (Some (new run_id r))
+					| (ClobbersTarget, [])  -> (rv.clobbers := true; None)
 					| (FileDependency, [mtime; cs; path]) ->
 							Some (new file_dependency
 								~mtime:(parse_mtime mtime)
@@ -350,6 +357,7 @@ and dependency_builder target_path (input:Lwt_io.input_channel) = object (self)
 			let rv = {
 				checksum = ref None;
 				run_id = ref None;
+				clobbers = ref false;
 				rules = ref [];
 			} in
 			lwt version_line = readline input in
@@ -434,6 +442,9 @@ and target_state (target_path:string) =
 					(fun output -> write_dependency output dep)
 			)
 
+		method mark_clobbers =
+			self#add_dependency (ClobbersTarget, [])
+
 		method add_file_dependency ~(mtime:Big_int.t option) ~(checksum:string option) path =
 			let dep = (new file_dependency ~mtime:mtime ~checksum:checksum path) in
 			log#trace "Adding dependency %s -> %a" (Filename.basename target_path) print_obj dep;
@@ -450,18 +461,16 @@ and target_state (target_path:string) =
 			log#trace "perform_build %s" self#path;
 
 			(* TODO: quicker mtime-based check *)
-			let still_needs_build = (fun () ->
+			let still_needs_build = (fun deps ->
 				log#trace "Checking if %s still needs buld after releasing lock" self#path;
-				lwt deps = self#deps in
-				Lwt.return @@ match deps with
+				match deps with
 					| Some d -> not d#already_built
 					| None -> true
 			) in
 
 			let build = (fun deps_path ->
-				lwt wanted = still_needs_build () in
-
-				if wanted then (
+				lwt deps = self#deps in
+				if still_needs_build deps then (
 					lwt builder_mtime = Util.get_mtime exe in
 					let builder_dep = new builder_dependency
 						~mtime: builder_mtime
@@ -477,7 +486,7 @@ and target_state (target_path:string) =
 						write_dependency file (serializable current_run_id)
 					) >>
 					lwt built = try_lwt
-						block exe
+						block exe deps
 					with ex ->
 						Lwt_unix.unlink temp >>
 						raise_lwt ex

ocaml/gup/state.mli

@@ -17,17 +17,19 @@ class target_state : string ->
 		method path : string
 
 		(* async methods *)
-		method perform_build : string -> (string -> bool Lwt.t) -> bool Lwt.t
+		method perform_build : string -> (string -> dependencies option -> bool Lwt.t) -> bool Lwt.t
 		method deps : dependencies option Lwt.t
 		method add_file_dependency : mtime:(Big_int.t option) -> checksum:(string option) -> string -> unit Lwt.t
 		method add_checksum : string -> unit Lwt.t
 		method mark_always_rebuild : unit Lwt.t
+		method mark_clobbers : unit Lwt.t
 	end
 
 and dependencies : string -> base_dependency intermediate_dependencies ->
 	object
 		method is_dirty : Gupfile.buildscript -> bool -> (target_state list) dirty_result Lwt.t
 		method checksum : string option
+		method clobbers : bool
 		method already_built : bool
 		method children : string list
 		method print : unit IO.output -> unit

python/gup/builder.py

@@ -86,9 +86,9 @@ def __repr__(self):
 		return 'Target(%r)' % (self.path,)
 
 	def build(self, update):
-		return self.state.perform_build(self.builder.path, lambda: self._perform_build(update))
+		return self.state.perform_build(self.builder.path, lambda deps: self._perform_build(update, deps))
 
-	def _perform_build(self, update):
+	def _perform_build(self, update, deps):
 		'''
 		Assumes locks are held (by state.perform_build)
 		'''
@@ -111,7 +111,6 @@ def _perform_build(self, update):
 
 		target_relative_to_cwd = os.path.relpath(self.path, ROOT_CWD)
 
-
 		output_file = os.path.abspath(self.state.meta_path('out'))
 		MOVED = False
 		try:
@@ -143,7 +142,10 @@ def _perform_build(self, update):
 				_log.trace("old_mtime=%r, new_mtime=%r" % (mtime, new_mtime))
 				if not os.path.isdir(self.path):
 					# directories often need to be created directly
-					_log.warn("%s modified %s directly" % (exe_path_relative_to_cwd, self.path))
+					self.state.mark_clobbers()
+					expect_clobber = False if deps is None else deps.clobbers
+					if not (update and expect_clobber):
+						_log.warn("%s modified %s directly" % (exe_path_relative_to_cwd, self.path))
 			if ret == 0:
 				if os.path.lexists(output_file):
 					if os.path.isdir(self.path) and not os.path.islink(self.path):

python/gup/state.py

@@ -85,15 +85,18 @@ def add_dependency(self, dep):
 			with open(self.meta_path('deps2'), 'a') as f:
 				dep.append_to(f)
 
+	def mark_clobbers(self):
+		self.add_dependency(ClobbersTarget())
+
 	def perform_build(self, exe, do_build):
 		assert os.path.exists(exe)
-		def still_needs_build():
+		def still_needs_build(deps):
 			_log.trace("checking if %s still neeeds build after releasing lock" % self.path)
-			deps = self.deps()
 			return deps is None or (not deps.already_built())
 
 		with self._ensure_dep_lock().write():
-			if not still_needs_build():
+			deps = self.deps()
+			if not still_needs_build(deps):
 				return False
 
 			builder_dep = BuilderDependency(
@@ -107,7 +110,7 @@ def still_needs_build():
 				Dependencies.init_file(f)
 				builder_dep.append_to(f)
 			try:
-				built = do_build()
+				built = do_build(deps)
 			except:
 				os.remove(temp)
 				raise
@@ -127,6 +130,7 @@ def __init__(self, path, file):
 		self.path = path
 		self.rules = []
 		self.checksum = None
+		self.clobbers = False
 		self.runid = None
 
 		if file is None:
@@ -149,6 +153,8 @@ def __init__(self, path, file):
 				elif isinstance(dep, RunId):
 					assert self.runid is None
 					self.runid = dep
+				elif isinstance(dep, ClobbersTarget):
+					self.clobbers = True
 				else:
 					self.rules.append(dep)
 
@@ -203,6 +209,7 @@ def parse(line):
 				AlwaysRebuild,
 				Checksum,
 				RunId,
+				ClobbersTarget,
 				BuildTime]:
 			if line.startswith(candidate.tag):
 				cls = candidate
@@ -385,3 +392,8 @@ def current(cls):
 
 	def is_current(self):
 		return self.value == RUN_ID
+
+class ClobbersTarget(Dependency):
+	tag = 'clobbers:'
+	num_fields = 0
+	fields = []

test/test_scripts.py

@@ -165,3 +165,27 @@ def test_permissions_of_tempfile_are_maintained(self):
 		self.build('hello')
 		out = subprocess.check_output(self.path('hello'))
 		self.assertEquals(out.strip().decode('ascii'), 'ok')
+
+	def test_ignores_repeated_clobbers_on_update_build(self):
+		self.touch('input')
+		self.write('bad.gup', BASH + 'gup -u input; echo bad > "$2"')
+		clobber_warning = '# WARNING bad.gup modified %s directly' % (os.path.join('.', 'bad'))
+
+		def warning(lines):
+			try:
+				return next(iter(filter(lambda line: line.startswith('# WARN'), lines)))
+			except StopIteration:
+				return None
+
+		# initial build should have the warning
+		lines = self.build_u('bad', include_logging=True)
+		self.assertEquals(warning(lines), clobber_warning)
+
+		# doesn't notify on rebuild
+		lines = self.assertRebuilds('bad', lambda: self.touch('input'), built=True, include_logging=True)
+		self.assertEquals(warning(lines), None)
+
+		# does notify on explicit build
+		lines = self.build('bad', include_logging=True)
+		self.assertEquals(warning(lines), clobber_warning)
+

test/util.py

@@ -46,7 +46,7 @@ def skipPermutations(fn):
 def has_feature(name):
 	return all([name in _build(exe, args=['--features'], cwd=None) for exe in GUP_EXES])
 
-def _build(exe, args, cwd, env=None):
+def _build(exe, args, cwd, env=None, include_logging=False):
 	env = env or os.environ
 	log.warn("\n\nRunning build with args: %r [cwd=%r]" % (list(args), cwd))
 	env = env.copy()
@@ -69,7 +69,8 @@ def _build(exe, args, cwd, env=None):
 		if not line:
 				break
 		line = line.decode('utf-8').rstrip()
-		if not line.startswith('#'): lines.append(line)
+		if include_logging or not line.startswith('#'):
+			lines.append(line)
 		unbuildable_msg = "Don't know how to build"
 		unbuildable_idx = line.find(unbuildable_msg)
 		if unbuildable_idx != -1:
@@ -152,13 +153,13 @@ def _root_cwd(self):
 		finally:
 			os.chdir(initial)
 
-	def _build(self, args, cwd=None, last=False, env=None):
+	def _build(self, args, cwd=None, last=False, env=None, include_logging=False):
 		self.invocation_count += 1
 		log.debug("invocation count = %r", self.invocation_count)
 
 		with self._root_cwd():
 			exe = next(self.exes)
-			lines = _build(exe, args=args, cwd=cwd, env=env)
+			lines = _build(exe, args=args, cwd=cwd, env=env, include_logging=include_logging)
 
 		if LAME_MTIME and not last:
 			# OSX has 1-second mtime resolution.
@@ -172,7 +173,7 @@ def _build(self, args, cwd=None, last=False, env=None):
 		return lines
 
 	def build(self, *targets, **k):
-		self._build(targets, **k)
+		return self._build(targets, **k)
 
 	def mtime(self, p):
 		mtime = os.stat(os.path.join(self.ROOT, p)).st_mtime
@@ -204,12 +205,13 @@ def completionTargets(self, dir=None):
 		if dir is not None: args.append(dir)
 		return sorted(self._build(args))
 
-	def assertRebuilds(self, target, fn, built=False):
-		if not built: self.build_u(target)
+	def assertRebuilds(self, target, fn, built=False, **kw):
+		if not built: self.build_u(target, **kw)
 		mtime = self.mtime(target)
 		fn()
-		self.build_u(target)
+		rv = self.build_u(target, **kw)
 		self.assertNotEqual(self.mtime(target), mtime, "target %s didn't get rebuilt" % (target,))
+		return rv
 
 	def assertDuration(self, min, max, fn):
 		initial_time = datetime.now()