Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: mistakenly changed sources to in-memory manifest #2272

Merged
merged 7 commits into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading