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

test: showing dependencies works in project.extract_manifest() #2236

Closed
wants to merge 1 commit into from

Conversation

antazoey
Copy link
Member

What I did

i heard there was a bug here but not sure what it is, here is a test proving dependencies works in package manifest.
lemme know how to add more conditions to reach a failing state

How I did it

How to verify it

Checklist

  • All changes are completed
  • New test cases have been added
  • Documentation has been updated


assert found, "Failed to find expected deployment URI"

# Prove that dependencies works.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are all local package dependencies?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but chatting with The God of Ninjas, I have learned to try stuff with snekmate, so going to experiment there.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be the case we just don't have a good way to share non-local dependencies using EthPM, and so we can just ask for additional dependencies when compiling in a more ape-friendly config format to use in his service

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as it's in your config, it should work, it just turns to a URL format.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it has to be the URL of an EthPM Manifest (which is not very common)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why just be a link to a directory containing the files? That is how it works now, and ape puts them in a manifest

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't really how it's meant to work, which is why EthPM needs to be fundamentally re-thought anyways. Dependencies are best as like links to a zip file, ipfs directory, or other filesystem-like resource that is globally reference-able and contains sources to use (and potentially compiler settings, compiled artifacts, and other reference metadata that may be useful too) so a downstream project can describe its build manifest in full

For the hosted-compiler we can just use a list of ape-compatible dependency objects which we can push into an ape-config to use to compile with

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, Ape is getting better at deducing settings from a project structure, such as Foundry projects.
So if you have a URL, you can fetch it and ape can do Project(path).extract_manifest()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

snekmate includes absolutely nothing besides contracts in their published PyPI package, which is frustrating. I told them that and they didn't care.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, a Python wheel is a zip file at some URI on the pypi server

@antazoey antazoey closed this Aug 22, 2024
@antazoey antazoey deleted the fix/missing-deps branch August 22, 2024 20:24
@antazoey antazoey restored the fix/missing-deps branch August 22, 2024 21:01
@antazoey antazoey deleted the fix/missing-deps branch September 16, 2024 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants