DavidVujic / poetry-multiproject-plugin

A Poetry plugin that makes it simple to share code between projects in monorepos.
https://davidvujic.github.io/python-polylith-docs/installation/
MIT License
190 stars 7 forks source link

One use-case for including packages breaks with --with-top-namespace=top_namespace #51

Closed randomgeek78 closed 8 months ago

randomgeek78 commented 8 months ago

I am running the following command to build my project:

poetry build-project --directory ./ --with-top-namespace=top_namespace

If my pyproject.toml has the following, things work as expected:

...
packages = [
    {include = "namespace/entrypoint",from = ".."},
]
...
[tool.poetry.scripts]
entrypoint = "namespace.entrypoint.task:run"

entrypoint package is now in top_namespace/namespace/entrypoint and the script points to top_namespace.namespace.entrypoint.task:run. Everything good!

But when I have the following in my pyproject.toml, things are not good:

...
packages = [
    {include = "namespace/entrypoint",from = "."},
]
...
[tool.poetry.scripts]
entrypoint = "namespace.entrypoint.task:run"

In this case, entrypoint is present in namespace/entrypoint but the script points to top_namespace.namespace.entrypoint.task:run.

Why does one setup work and the other does not?

Also, more surprisingly, this works-

...
packages = [
    {include = "namespace/entrypoint",from = "../project"},
]
...
[tool.poetry.scripts]
entrypoint = "namespace.entrypoint.task:run"

But from="../project" and from="." point to the same directory!

DavidVujic commented 8 months ago

That looks like a new bug that I have introduced: it should be either-or, both changed or none changed.

The case where the from is a path that is not relative upwards, the script is written to not do any changes because the use case this is meant for is to "fix" the relative paths and their namespaces. Any source code that is within the current project boundaries (i.e. subfolders of the pyproject.toml) is within the project's own control.

But it shouldn't have changed the script entry point there, so that's a bug.

DavidVujic commented 8 months ago

The more I think about this, I realize that I might not have built this new feature properly. As with the latest version, it is assumed that the entry point is one of the rewritten modules. But users of this plugin - at least those not using Polylith - might have a different structure and have an entrypoint located within the project boundaries. In that scenario, the change will break their builds. 🤦

I think I will have to revert the changes made in the latest version.

randomgeek78 commented 8 months ago

I completely understand. Thank you for explaining the internals so nicely. :pray:

Speaking as a user of the module, for me, --with-top-namespace=top_namespace should put all artifacts built by the project into the top_namespace - that includes artifacts created internal to the project and those pulled from outside (relatively up the directory structure). The pyproject.toml should have no knowledge and be completely decoupled from top_namespace. That includes the scripts in pyproject.toml which too should have no knowledge of top_namespace. That also means that when we do poetry build-project using --with-top-namespace=top_namespace, all internal artifacts, relative artifacts and scripts need to be moved.

That, to me, really makes --with-top-namespace=top_namespace truly powerful.

The mental model is that, poetry build-project should create a perfectly consistent, working wheel with and without --with-top-namespace=top_namespace with complete separation between the two.

The way I use --with-top-namespace=top_namespace currently is to create sandboxed builds where the components from two builds don't interfere with each other because they live in separate top_namespace paths within the site-packages. I also have a build where I don't use --with-top-namespace=top_namespace for development.

To me, what we have currently accomplishes most of it except for the weird from= thingy.

I should also note that I tried playing around with the to=".." option that is also available. Maybe that is the right way to move things around to build something consistent and the --with-top-namespace=top_namespace is an abstraction over that to control where the entire set of artifacts built land in site-packages (including scripts and relative components and internal modules/packages).

DavidVujic commented 8 months ago

I think I have found a way to solve parts of this:

currently working on a solution that will only rewrite the script entry point if there is a matching relative path. This should prevent a rewrite in the case where the entry point is a local module and not in the package includes that are located upwards in the hiearchy.

Your workaround with the ".." should still be applicable though 😄

DavidVujic commented 8 months ago

I think that the single . scenario is solved with the just released 1.4.1 version. In that release, the rewrite is only happening when there are relative package includes that matches the namespace of the script entry point.

The trick to add ".." will still work though, and I will leave that as-is.

Please reopen this if things doesn't work as expected!

randomgeek78 commented 8 months ago

Hi David, Thanks for arriving at this solution. Your most recent commit - we were able to make it work for us! Thanks!