angr / angr-dev

Some helper scripts to set up an environment for angr development.
BSD 2-Clause "Simplified" License
114 stars 95 forks source link

I want to be able to `pip install -e .` angr/archr/angr-management/etc #113

Closed ConnorNelson closed 2 years ago

ConnorNelson commented 3 years ago

I want to be able to pip install -e . angr/archr/angr-management/etc.

This does not work because version 9.0.gitrolling is a lie. It does not actually exist. I think the gitrolling concept makes sense to me, but should be implemented differently so that setup.py still works. This could be done for instance, via a __gitrolling__ = ["angr"] assignment that gets parsed out during CI release.

Here is a rough prototype of what could be used to accomplish this:

import ast

def patch_gitrolling(version):
    with open("setup.py") as f:
        setup = ast.parse(f.read())

    for node in setup.body:
        if not isinstance(node, ast.Assign):
            continue
        if not any(target.id == "__gitrolling__" for target in node.targets):
            continue
        if not hasattr(node.value, "elts"):
            continue
        gitrolling = [e.value for e in node.value.elts]
        break
    else:
        gitrolling = []

    for node in setup.body:
        if not isinstance(node, ast.Expr):
            continue
        if not isinstance(node.value, ast.Call):
            continue
        if not isinstance(node.value.func, ast.Name):
            continue
        if not node.value.func.id == "setup":
            continue
        for keyword in node.value.keywords:
            if keyword.arg != "install_requires":
                continue
            if not hasattr(keyword.value, "elts"):
                continue
            for e in keyword.value.elts:
                if e.value in gitrolling:
                    e.value = f"{e.value}=={version}"

    return ast.unparse(setup)

print(patch_gitrolling(version="9.0.gitrolling"))
ltfish commented 3 years ago

This does not work because version 9.0.gitrolling is a lie.

Question: Why doesn't it work for you? Are you trying to mix git versions and pypi versions of angr components? Don't do that.

ConnorNelson commented 3 years ago

The issue is that there isn't a good way to install the git versions. My example yesterday: I clone angr management, and run pip install -e .. This fails because there is no angr==9.0.gitrolling available. Now it sounds like what you're stating is "well why don't you just git clone angr, and pip install ." which will result in an angr==9.0.gitrolling on my system. But that won't work because angr depends on gitrolling versions as well.

Now maybe that's where setup.sh comes into play. But that insane. I shouldn't need to pull down and run an installer script. This is 2021. We have the technology to make my pip install work.

If we don't want to mix git with pypi, then I propose (and I think I'm stealing this idea from what @twizmwazin briefly mentioned to me a week ago) that the default setup.py point to angr @ git+https://github.com/angr/angr.

I don't think it's okay to have a setup.py which when run, just fails and doesn't resolve and pull down dependencies as is half the purpose of a setup.py.

ltfish commented 3 years ago

Now it sounds like what you're stating is "well why don't you just git clone angr, and pip install ." which will result in an angr==9.0.gitrolling on my system. But that won't work because angr depends on gitrolling versions as well.

I'm stating that you need to install angr and all its dependencies (that we control and release together with angr) from GitHub to avoid mixing GitHub versions and PyPI versions.

To make this clear: You should install all angr dependencies from GitHub (thus, having 9.0.gitrolling for all of them) if you choose to use a development install.

But that insane. I shouldn't need to pull down and run an installer script. This is 2021. We have the technology to make my pip install work.

I still think mixing GitHub versions and PyPI versions is a really bad idea. You are just inviting problems by proposing seemingly smart ideas. They are not.

I don't think it's okay to have a setup.py which when run, just fails and doesn't resolve and pull down dependencies as is half the purpose of a setup.py.

In my understanding, the setup.py in PyPI releases has correct version numbers: We update setup.py as we release to PyPI. We switched to .gitrolling specifically because we want to avoid the mixing of GitHub versions and PyPI versions.

However, I do see your point, and I agree that it would be better if the setup.py of angr management development install can automatically pull down and replace your local dependencies. Will that work if your existing angr dependencies are from PyPI? I doubt it. @twizmwazin what do you think?

ltfish commented 3 years ago

The issue is that there isn't a good way to install the git versions.

Use setup.sh on Linux. You can also manually clone everything and run pip install -e . for each of them in the correct order: archinfo, pyvex, claripy, ailment, cle, and angr.

twizmwazin commented 3 years ago

We should be able to both require 9.0.gitrolling for dependencies, and then reference github as the source of that version, allowing something like pip install -e https://github.com/angr/angr-management.git to work correctly. Doing this would require a tweak to the release pipeline to later remove that for PyPI releases, but likely nothing too crazy.

However, I think there is a big caveat here in that supporting this would likely lead to users not updating angr dependencies in sync with the package they are developing on, since pip would see the outdated, already installed versions as satisfactory. Solving that properly requires having some sort of API stability and moving away from the "monorepo but in lots of repos" workflow, but we can ignore that for now and accept and deal with github issues as they arise. Ignoring the API stability option, our choice is between having occasional issues like this, or having occasional "I installed x from github, updated only that package, and now it is broken" issues.

ltfish commented 3 years ago

@twizmwazin that sounds OK to me.

ConnorNelson commented 3 years ago

@zardus

zardus commented 3 years ago

Summarizing a conversation being held here in DC: it seems that our choice of this gitrolling method deprioritizes the convenience of those that know what they're doing under the convenience of those that don't know what they're doing. This seems like a suboptimal tradeoff: we should make things usable for our researchers first and foremost.

Can't we just make the azure pipeline just update the dependencies to the actual versions?

github-actions[bot] commented 2 years ago

This issue has been marked as stale because it has no recent activity. Please comment or add the pinned tag to prevent this issue from being closed.

github-actions[bot] commented 2 years ago

This issue has been marked as stale because it has no recent activity. Please comment or add the pinned tag to prevent this issue from being closed.

github-actions[bot] commented 2 years ago

This issue has been closed due to inactivity.