Skip to content

Commit

Permalink
Fixup: improper use of shouldRebuild uppon cp/mv
Browse files Browse the repository at this point in the history
  • Loading branch information
drakes00 committed Mar 13, 2024
1 parent ece1d91 commit 993fcf0
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 12 deletions.
Binary file modified .coverage
Binary file not shown.
4 changes: 3 additions & 1 deletion doc/builders.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@ generic. Their inputs and outputs are precised by rules.

```python
class Builder:
def __init__(self, action, ephemeral=False):
def __init__(self, action, ephemeral=False, shouldRebuild=None):
"""
Initialize the Builder.
Parameters:
- `action`: The action to be performed when building the targets.
- `ephemeral` (optional): If set to `True`, the builder will not be registered, making it suitable for one-time use.
- `shouldRebuild` (optional): Can be set to a callable(target, deps) -> bool, customizing how dependencies and targets are linked.
"""
pass
```
Expand Down Expand Up @@ -74,6 +75,7 @@ the dependencies to the targets.
def do_some_work(deps, targets, console, myArg=None):
# Any python code producing `targets` from `deps`.
# Keyword `myArg` may be used depending on the rule.
pass

myBuilder = Builder(action=do_some_work)
Rule(targets="output.txt", deps="input.txt", builder=myBuilder)
Expand Down
19 changes: 17 additions & 2 deletions remake/builders.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,21 @@
# ==================================================


def _FILE_OPS_shouldRebuild(target, deps):
from remake.main import shouldRebuild

if len(deps) > 1:
ret = False
for dep in deps:
ret = ret or shouldRebuild(target / dep.name, [dep])
return ret
else:
dep = deps[0]
if target.is_dir():
target = target / dep.name
return shouldRebuild(target, [dep])


# Expects either :
# - 2 arguments (source, destination):
# - If source is a file and dest does not exists -> Ok, rename as dest
Expand Down Expand Up @@ -49,7 +64,7 @@ def _cp(deps, targets, _):
shutil.copytree(dep, target)


cp = Builder(action=_cp)
cp = Builder(action=_cp, shouldRebuild=_FILE_OPS_shouldRebuild)


# Expects either :
Expand All @@ -73,7 +88,7 @@ def _mv(deps, targets, _):
shutil.move(dep, targets[0])


mv = Builder(action=_mv)
mv = Builder(action=_mv, shouldRebuild=_FILE_OPS_shouldRebuild)


def _rm(deps, targets, _):
Expand Down
38 changes: 29 additions & 9 deletions remake/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import os
import pathlib
import re
import shutil
import subprocess
import sys

Expand Down Expand Up @@ -167,7 +168,7 @@ def shouldRebuild(target: VirtualTarget | pathlib.Path, deps: list[VirtualDep |
if isinstance(target, VirtualTarget):
# Target is virtual, always rebuild.
return True
if not os.path.isfile(target):
if not os.path.exists(target):
# If target does not already exists.
return True
else:
Expand All @@ -188,19 +189,24 @@ def shouldRebuild(target: VirtualTarget | pathlib.Path, deps: list[VirtualDep |
class Builder():
"""Generic builder class."""
_action = None
_shouldRebuild = None

def __init__(
self,
action: list[str] | str | Callable[[list[str],
list[str],
Console],
None],
ephemeral: bool = False
ephemeral: bool = False,
shouldRebuild: Callable[[VirtualTarget | pathlib.Path,
list[VirtualDep | pathlib.Path]],
bool] | None = None,
):
if isinstance(action, str):
self._action = action.split(" ")
else:
self._action = action
self._shouldRebuild = shouldRebuild
if not ephemeral:
self._register()

Expand Down Expand Up @@ -249,9 +255,14 @@ def action(self) -> Callable[[list[str], list[str], Console], None]:

@property
def type(self):
"""Returns build's action's type (list vs. callable)."""
"""Returns builder's action's type (list vs. callable)."""
return type(self._action)

@property
def shouldRebuild(self):
"""Returns buider's custom shouldRebuild function."""
return self._shouldRebuild


@typechecked()
class Rule():
Expand Down Expand Up @@ -345,8 +356,14 @@ def apply(self, console: Console | Progress | None = None) -> bool:
"""

# Check if rule is already applied (all targets are already made).
if all(not shouldRebuild(target, self._deps) for target in self._targets):
return False
if self._builder.shouldRebuild:
# Either with custom shouldRebuild method.
if all(not self._builder.shouldRebuild(target, self._deps) for target in self._targets):
return False
else:
# Or using default one.
if all(not shouldRebuild(target, self._deps) for target in self._targets):
return False

# If we are not in dry run mode, ensure dependencies were made before the rule is applied.
if not DRY_RUN:
Expand Down Expand Up @@ -620,7 +637,7 @@ def findBuildPath(
}

# At this point, no rule was found for the target.
if os.path.isfile(str(target)):
if os.path.exists(str(target)):
# And target already exists.
if CLEAN:
# We are attempting to clean an existing target no linked to any rule.
Expand Down Expand Up @@ -757,11 +774,14 @@ def cleanDeps(deps: list[pathlib.Path | tuple[pathlib.Path,
Rule]]:
"""Builds files marked as targets from their dependencies."""
def _cleanDep(target):
if os.path.isfile(target):
if os.path.exists(target):
progress.console.print(
f"[{job+1}/{len(deps)}] [[bold plum1]CLEAN[/bold plum1]] Cleaning dependency {target}."
)
os.remove(target)
if target.is_file():
os.remove(target)
elif target.isdir():
shutil.rmtree(target)

with Progress() as progress:
progress.console.print(
Expand Down Expand Up @@ -808,7 +828,7 @@ def buildDeps(deps: list[pathlib.Path | tuple[pathlib.Path,
progress.console.print(
f"[{job+1}/{len(deps)}] [[bold plum1]DRY-RUN[/bold plum1]] Dependency: {dep}"
)
elif os.path.isfile(dep):
elif os.path.exists(dep):
progress.console.print(
f"[{job+1}/{len(deps)}] [[bold plum1]SKIP[/bold plum1]] Dependency {dep} already exists."
)
Expand Down

0 comments on commit 993fcf0

Please sign in to comment.