Skip to content

Commit

Permalink
fix: mistakenly changed sources to in-memory manifest (#2272)
Browse files Browse the repository at this point in the history
  • Loading branch information
antazoey authored Sep 10, 2024
1 parent 9eaa07c commit 203e63d
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 41 deletions.
62 changes: 33 additions & 29 deletions src/ape/managers/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -2125,34 +2125,37 @@ def __getattr__(self, item: str) -> Any:
message = getattr(err, "message", str(err))
did_append = False

all_files = get_all_files_in_directory(self.contracts_folder)
for path in all_files:
# Possibly, the user was trying to use a file name instead.
if path.stem != item:
continue
if item not in (self.manifest.contract_types or {}):
all_files = get_all_files_in_directory(self.contracts_folder)
for path in all_files:
# Possibly, the user was trying to use a file name instead.
if path.stem != item:
continue

if message and message[-1] not in (".", "?", "!"):
message = f"{message}."
message = (
f"{message} However, there is a source file named '{path.name}', "
"did you mean to reference a contract name from this source file?"
)
did_append = True
break

# Possibly, the user does not have compiler plugins installed or working.
missing_exts = set()
for path in all_files:
if ext := get_full_extension(path):
if ext not in self.compiler_manager.registered_compilers:
missing_exts.add(ext)

if missing_exts:
start = "Else, could" if did_append else "Could"
message = (
f"{message} {start} it be from one of the "
"missing compilers for extensions: " + f'{", ".join(sorted(missing_exts))}?'
)
if message and message[-1] not in (".", "?", "!"):
message = f"{message}."

message = (
f"{message} However, there is a source file named '{path.name}'. "
"This file may not be compiling (see error above), or maybe you meant "
"to reference a contract name from this source file?"
)
did_append = True
break

# Possibly, the user does not have compiler plugins installed or working.
missing_exts = set()
for path in all_files:
if ext := get_full_extension(path):
if ext not in self.compiler_manager.registered_compilers:
missing_exts.add(ext)

if missing_exts:
start = "Else, could" if did_append else "Could"
message = (
f"{message} {start} it be from one of the "
"missing compilers for extensions: " + f'{", ".join(sorted(missing_exts))}?'
)

err.args = (message,)
raise # The same exception (keep the stack the same height).
Expand Down Expand Up @@ -2356,9 +2359,10 @@ def isolate_in_tempdir(self, **config_override) -> Iterator["LocalProject"]:
yield self

else:
# Add sources to manifest memory, in case they are missing.
self._manifest.sources = dict(self.sources.items())
sources = dict(self.sources.items())
with super().isolate_in_tempdir(**config_override) as project:
# Add sources to manifest memory, in case they are missing.
project.manifest.sources = sources
yield project

def unpack(self, destination: Path, config_override: Optional[dict] = None) -> "LocalProject":
Expand Down
10 changes: 1 addition & 9 deletions src/ape_pm/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,7 @@ def compile(

code = src_path.read_text()
source_id = source_ids[path]
try:
# NOTE: Always set the source ID to the source of the JSON file
# to avoid manifest corruptions later on.
contract_type = self.compile_code(code, project=project, sourceId=source_id)
except CompilerError as err:
logger.warning(
f"Unable to parse {ContractType.__name__} from '{source_id}'. Reason: {err}"
)
continue
contract_type = self.compile_code(code, project=project, sourceId=source_id)

# NOTE: Try getting name/ ID from code-JSON first.
# That's why this is not part of `**kwargs` in `compile_code()`.
Expand Down
26 changes: 23 additions & 3 deletions tests/functional/test_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,25 @@ def test_isolate_in_tempdir(project):
assert not tmp_project.manifest_path.is_file()


def test_isolate_in_tempdir_does_not_alter_sources(project):
# First, create a bad source.
with project.temp_config(contracts_folder="tests"):
new_src = project.contracts_folder / "newsource.json"
new_src.write_text("this is not json, oops")
project.sources.refresh() # Only need to be called when run with other tests.

try:
with project.isolate_in_tempdir() as tmp_project:
# The new (bad) source should be in the temp project.
assert "tests/newsource.json" in (tmp_project.manifest.sources or {}), project.path
finally:
new_src.unlink()
project.sources.refresh()

# Ensure "newsource" did not persist in the in-memory manifest.
assert "tests/newsource.json" not in (project.manifest.sources or {})


def test_in_tempdir(project, tmp_project):
assert not project.in_tempdir
assert tmp_project.in_tempdir
Expand Down Expand Up @@ -200,8 +219,9 @@ def test_getattr_same_name_as_source_file(project_with_source_files_contract):
expected = (
r"'LocalProject' object has no attribute 'ContractA'\. "
r"Also checked extra\(s\) 'contracts'\. "
r"However, there is a source file named 'ContractA\.sol', "
r"did you mean to reference a contract name from this source file\? "
r"However, there is a source file named 'ContractA\.sol'\. "
r"This file may not be compiling \(see error above\), "
r"or maybe you meant to reference a contract name from this source file\? "
r"Else, could it be from one of the missing compilers for extensions: .*"
)
with pytest.raises(AttributeError, match=expected):
Expand Down Expand Up @@ -587,7 +607,7 @@ def test_project_api_foundry_and_ape_config_found(foundry_toml):

def test_get_contract(project_with_contracts):
actual = project_with_contracts.get_contract("Other")
assert isinstance(actual, ContractContainer)
assert isinstance(actual, ContractContainer), f"{type(actual)}"
assert actual.contract_type.name == "Other"

# Ensure manifest is only loaded once by none-ing out the path.
Expand Down
27 changes: 27 additions & 0 deletions tests/integration/cli/test_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,33 @@ def test_compile_when_sources_change(ape_cli, runner, integ_project, clean_cache
assert "contracts/Interface.json" not in result.output


@skip_projects_except("multiple-interfaces")
def test_compile_when_sources_change_problematically(ape_cli, runner, integ_project, clean_cache):
"""
There was a bug when sources changes but had errors, that the old sources continued
to be used and the errors were swallowed.
"""
source_path = integ_project.contracts_folder / "Interface.json"
content = source_path.read_text()
assert "bar" in content, "Test setup failed - unexpected content"

result = runner.invoke(
ape_cli, ("compile", "--project", f"{integ_project.path}"), catch_exceptions=False
)
assert result.exit_code == 0, result.output

# Change the contents of a file in a problematic way.
source_path = integ_project.contracts_folder / "Interface.json"
modified_source_text = source_path.read_text().replace("{", "BRACKET")
source_path.unlink()
source_path.touch()
source_path.write_text(modified_source_text, encoding="utf8")
result = runner.invoke(
ape_cli, ("compile", "--project", f"{integ_project.path}"), catch_exceptions=False
)
assert result.exit_code != 0, result.output


@skip_projects_except("multiple-interfaces")
def test_compile_when_contract_type_collision(ape_cli, runner, integ_project, clean_cache):
source_path = integ_project.contracts_folder / "Interface.json"
Expand Down

0 comments on commit 203e63d

Please sign in to comment.