ApeWorX / ape-vyper

Vyper compiler plugin for the Ape Framework, using VVM
https://www.apeworx.io/
Apache License 2.0
24 stars 9 forks source link

feat: vyper 0.4 support #110

Closed antazoey closed 3 weeks ago

antazoey commented 2 months ago

What I did

Make it so can compile on 0.4.0

How I did it

Had to use absolute paths.

How to verify it

Checklist

antazoey commented 2 months ago

Is this checked against 0.3.10 and 0.4.0?

Sorry meant to make this a draft.. The tests are all pre-0.4.0, but I need to add more tests for 0.4.0 range.

charles-cooper commented 2 months ago

i think rather than using absolute paths, you can add the "./" to the search path

antazoey commented 2 months ago

i think rather than using absolute paths, you can add the "./" to the search path

I tried this but was running into other issues, I can keep trying though but I was in a tangled web of confusion.

antazoey commented 2 months ago

Looks like vyper in the 0.3.0 range does not like the absolute paths

pcaversaccio commented 2 months ago

currently it fails to process .vyi files image

pcaversaccio commented 2 months ago

Looks like imports don't resolve yet:

image

antazoey commented 2 months ago

WIP - still in development, please be patient.

pcaversaccio commented 2 months ago

FYI @fubuloubu, it doesn't work yet on snekmate: https://github.com/pcaversaccio/snekmate/actions/runs/8651807519/job/23771914522#step:8:1 Screenshot_20240413_081527_GitHub.jpg

charles-cooper commented 2 months ago

i think constructing the input dict from just a directory is going to be pretty complicated, because it's hard to predict how the compiler will interact with search path (cf. for instance: vyperlang/vyper@b0ea5b6f1c8cd8d09db6f37e9857f9b3837fb386, vyperlang/vyper@d3723783caf3edfd8905fea0b221f99f18eeb27b, vyperlang/vyper@c6f457a73db40e4b113497883bd330e0dcec28d1, vyperlang/vyper@a3bc3eb50ea10788a688ea79d74d294cd9a418d6, vyperlang/vyper@5d10ea0d2a26ab0c58beab4b0b9a4a3d90c9c439).

i'm not sure what direction ape wants to take here, but maybe the best thing might be to call the compiler and recursively get its ast output (per vyperlang/vyper@9428196a0c48ec7cc02938ee4ae1a0e2fee133c8), or just use the cli and get -f combined_json or something. we could do something else too like adding -f import_graph as a compiler output option or something, i'm open to suggestions.

fubuloubu commented 2 months ago

we could do something else too like adding -f import_graph as a compiler output option or something, i'm open to suggestions.

Having the import graph would be extremely useful, in both directions (know which code modules import which other code modules so that a downstream module change will trigger us to recompile one that hasn't because of the dependency)

charles-cooper commented 2 months ago

@fubuloubu also check -f annotated_ast output, it contains information about the imported modules including the sha256sum of the content. you can use this to compute a reproducible import graph. https://github.com/vyperlang/vyper/pull/3891 goes further and includes an integrity hash output, which is the recursive hash of the file and all its dependencies. in vyperlang/vyper#3891 we could maybe add a json output, i'm not sure if that's very helpful here (since in any case we have to run in "regular" non-json mode first to grab the import graph).

maybe the best thing is to ditch standard-json input here? because it's not trivial to construct. from CLI, everything just works since the compiler will resolve things in the filesystem automatically.

if you want to keep using standard-json and one-shot the compilation (i.e., not have to query the compiler multiple times about a file), the other way would be to bundle everything ape knows is in the environment (hypothetical example: shove contracts/, libraries/, modules/) all into json input. that provides the best chance of successful compilation, although you still might end up getting divergences between what the compiler thinks is in the environment and what ape thinks is in the environment.

antazoey commented 2 months ago

@pcaversaccio I promise I will make sure snekmate compiles before we merge this.

fubuloubu commented 2 months ago

@fubuloubu also check -f annotated_ast output, it contains information about the imported modules including the sha256sum of the content. you can use this to compute a reproducible import graph. vyperlang/vyper#3891 goes further and includes an integrity hash output, which is the recursive hash of the file and all its dependencies. in vyperlang/vyper#3891 we could maybe add a json output, i'm not sure if that's very helpful here (since in any case we have to run in "regular" non-json mode first to grab the import graph).

maybe the best thing is to ditch standard-json input here? because it's not trivial to construct. from CLI, everything just works since the compiler will resolve things in the filesystem automatically.

if you want to keep using standard-json and one-shot the compilation (i.e., not have to query the compiler multiple times about a file), the other way would be to bundle everything ape knows is in the environment (hypothetical example: shove contracts/, libraries/, modules/) all into json input. that provides the best chance of successful compilation, although you still might end up getting divergences between what the compiler thinks is in the environment and what ape thinks is in the environment.

We basically want a way to know that we don't need to even call vyper in the first place because none of the files (or their dependencies) have made an update which should change their output.

Since subprocessing out to vyper is more expensive than not, having the checksum information in Ape's project metadata helps us avoid unnecessary extra compiler invocations

It is also still valuable that vyper does this check to, but would be great if that information were made accessible back to how Ape stores it's own project cache (which may include multiple compilers to check for)

antazoey commented 2 months ago

Let's see if this is even a problem first as well. This is still in development, but getting closer each day. If we can handle Solidity, I think we can handle Vyper.

fubuloubu commented 2 months ago

Let's see if this is even a problem first as well. This is still in development, but getting closer each day. If we can handle Solidity, I think we can handle Vyper.

0.4.0 made a big shift to absolute file locations from std json input, this is very different than how solidity or vyper (in the past) has handled file inputs

but yeah, calling annotated_ast (if available) sounds like it will give us the checksum information we can store in our own metadata cache. I missed the integrity hash concept, but since we only add back references for dependencies to be able to determine this, it could also help. That was non-standard in our EthPM v3 implementation

charles-cooper commented 2 months ago

We basically want a way to know that we don't need to even call vyper in the first place because none of the files (or their dependencies) have made an update which should change their output.

Since subprocessing out to vyper is more expensive than not, having the checksum information in Ape's project metadata helps us avoid unnecessary extra compiler invocations

since dependencies can change even if source code (of the compilation target) has not changed, there are two surefire ways to do this. one is going to be to call the vyper compiler and get the integrity hash (this only runs the frontend analysis passes, which based on latest benchmarks can process roughly 1000 lines of source code per second). one thing i can do (and i was thinking about doing this anyway) is to move the compiler's import analysis into an even earlier phase, so basically the only work the compiler does to get the integrity hash is resolving imports + hashing. but it would be a bit nontrivial to implement.

the other way is to package all the project files into a single package (like standard json input) and check that nothing in the package has changed.

fubuloubu commented 2 months ago

one thing i can do (and i was thinking about doing this anyway) is to move the compiler's import analysis into an even earlier phase, so basically the only work the compiler does to get the integrity hash is resolving imports + hashing. but it would be a bit nontrivial to implement.

we essentially do import analysis to build the graph of the project, so this would be really nice to expose. we may in the future execute multiple compiler processes in parallel if we determine (based on the graph) that there are disjoint sets within the project (allowing us to parallelize compiler invocations). an obvious first version of this would be to just parallelize invocations for vyper and solidity files within the same project

charles-cooper commented 2 months ago

we essentially do import analysis to build the graph of the project, so this would be really nice to expose. we may in the future execute multiple compiler processes in parallel if we determine (based on the graph) that there are disjoint sets within the project (allowing us to parallelize compiler invocations). an obvious first version of this would be to just parallelize invocations for vyper and solidity files within the same project

there isn't really work-sharing between different compilation targets. like if X imports A, and Y imports A, each one will re-compile A from scratch. so you can just always parallelize compilation targets.

fubuloubu commented 2 months ago

we essentially do import analysis to build the graph of the project, so this would be really nice to expose. we may in the future execute multiple compiler processes in parallel if we determine (based on the graph) that there are disjoint sets within the project (allowing us to parallelize compiler invocations). an obvious first version of this would be to just parallelize invocations for vyper and solidity files within the same project

there isn't really work-sharing between different compilation targets. like if X imports A, and Y imports A, each one will re-compile A from scratch. so you can just always parallelize compilation targets.

how do we know what the targets are?

charles-cooper commented 3 weeks ago

we essentially do import analysis to build the graph of the project, so this would be really nice to expose. we may in the future execute multiple compiler processes in parallel if we determine (based on the graph) that there are disjoint sets within the project (allowing us to parallelize compiler invocations). an obvious first version of this would be to just parallelize invocations for vyper and solidity files within the same project

there isn't really work-sharing between different compilation targets. like if X imports A, and Y imports A, each one will re-compile A from scratch. so you can just always parallelize compilation targets.

how do we know what the targets are?

oh i missed this -- targets are whatever the user asks for. (anything in contracts/?)

fubuloubu commented 3 weeks ago

we essentially do import analysis to build the graph of the project, so this would be really nice to expose. we may in the future execute multiple compiler processes in parallel if we determine (based on the graph) that there are disjoint sets within the project (allowing us to parallelize compiler invocations). an obvious first version of this would be to just parallelize invocations for vyper and solidity files within the same project

there isn't really work-sharing between different compilation targets. like if X imports A, and Y imports A, each one will re-compile A from scratch. so you can just always parallelize compilation targets.

how do we know what the targets are?

oh i missed this -- targets are whatever the user asks for. (anything in contracts/?)

we don't require the user to manually specify their targets, typically we try to identify the most efficient way to compile everything so that when the user goes and asks for a specific target (e.g. project.MyContractTarget) that target is already compiled and available for immediate use.

requiring the user to specify all of their targets strikes me as poor developer experience.

pcaversaccio commented 3 weeks ago

So I tested the latest commit and snekmate contracts compile: https://github.com/pcaversaccio/snekmate/pull/244.

Two notes:

  1. When installing Ape, Ape pulls in the old Vyper 0.3.x series (example):
Collecting vyper~=0.3.7 (from ape-vyper==0.1.0b3.dev69+g6154453)
  Downloading vyper-0.3.10-py3-none-any.whl.metadata (6.7 kB)

This is unfortunate since I need to uninstall Vyper and reinstall the new Vyper later again (otherwise I have Vyper 0.3.x installed locally or in the CI). It would be great to have an option to disable this installation step (@antazoey explained to me why this is currently needed for the flattener).

  1. @charles-cooper: in the ape-config.yaml of my PR you can see how I set contracts_folder: src/snekmate and exclude: r"(?!.*_mock\.vy$)", which can be used to isolate what is being compiled by Ape. Also, since 0.8.0, the default base path for compiling projects is now the project root instead of the contracts/ folder.
fubuloubu commented 3 weeks ago
  1. When installing Ape, Ape pulls in the old Vyper 0.3.x series (example):
Collecting vyper~=0.3.7 (from ape-vyper==0.1.0b3.dev69+g6154453)
  Downloading vyper-0.3.10-py3-none-any.whl.metadata (6.7 kB)

This is unfortunate since I need to uninstall Vyper and reinstall the new Vyper later again (otherwise I have Vyper 0.3.x installed locally or in the CI). It would be great to have an option to disable this installation step (@antazoey explained to me why this is currently needed for the flattener).

We should probably only have a lower pin for the flattener CLI, or as low a pin as we can manage

Wonder if it makes sense as an extra (and don't add flattener CLI if vyper not installed?) Spitballing here

antazoey commented 3 weeks ago

@fubuloubu

Wonder if it makes sense as an extra (and don't add flattener CLI if vyper not installed?) Spitballing here

This was my thought as well.

Another option I was going to look into was check what was being imported from vyper and see if i can copy it in manually (provided it is the same across all versions). It would save a lot of headache and the code won't change much...

Either way, can we do this in another PR?

fubuloubu commented 3 weeks ago

Either way, can we do this in another PR?

sounds good, there is a workaround for now (just re-install vyper afterwards)

Another option I was going to look into was check what was being imported from vyper and see if i can copy it in manually (provided it is the same across all versions). It would save a lot of headache and the code won't change much...

can you mark this in an issue?

antazoey commented 3 weeks ago

can you mark this in an issue?

https://github.com/ApeWorX/ape-vyper/issues/114