install: add warning if current project cannot be installed

[radoering]

Companion to python-poetry/poetry-core#629

• ModuleOrPackageNotFound is now raised in EditableBuilder.build() instead of EditableBuilder.__init__()
• The existence of a specified readme is now checked before the existence of the module. -> Just not declare a non-existing readme in the test.

Update: In the second commit, I changed the current behavior so that errors are no longer swallowed silently when the installation of the current project fails. I only added a warning and did not change the return code to avoid a breaking change. We can decide if we only want to go with the first commit or both commits.

Pro second commit: Currently, the user will not know if installing the current project succeeds or fails.

Contra second commit: Users who don't use --no-root and don't care about the current project being installed or not, might be annoyed by the warning.

[rragundez]

Can you update the branch? I guess that will give it more chance to be reviewed

[rragundez]

Hi @Secrus can you have a look at this one as well? python-poetry/poetry-core#629

[silverwind]

I'm confused about this new warning. What does "install the current project" mean in practice? Does it copy or symlink the current project's files into the virtualenv so that it can be imported by name?

If above is true, I think this behaviour should be removed and users should just be required to install a self-dependency, eliminating this confusing warning for users who don't need this feature.

[radoering]

It does an editable install. More specifically, it creates a *.pth file in the virtualenv, which probably is comparable to a symlink.

The warning means that your package is probably broken. If you don't care and don't like to add --no-root, you are probably interested in a "non-package" mode as described in #1132.

[silverwind]

Yes, my "package" contains no code, it's only used to run dependencies via poetry run. I guess I will run with --no-root until #1132 is fixed.

[adriangb]

Should this be triggered if packages = [] in pyproject.toml? I'd think not but it seems to.

[radoering]

packages = [] is not a reliable indicator to determine if this check should be triggered. If it's empty, there's a default. I think the only way is to decide is an explicit attribute, see #1132.

[adriangb]

I'm surprised by that. I'd think if the user explicitly specifies packages = [] then Poetry should respect that and only use a default if no packages is supplied at all. That's how it seemed to work from a users perspective prior to this PR (if Poetry tried to install some default and failed silently I don't know). In fact the docs said and currently say:

Using packages disables the package auto-detection feature meaning you have to explicitly specify the “default” package.

[radoering]

Using packages disables the package auto-detection feature meaning you have to explicitly specify the “default” package.

I suppose the author of this part of the docs did not expect that somebody interprets "using packages" as setting it to an empty list. 😄

I'd think if the user explicitly specifies packages = [] then Poetry should respect that

On the one hand "yes", but on the other hand: does it make sense to build a distribution without any package/module or do you only use packages = [] if you never intend to build an sdist/wheel?

In the latter case, I think it's probably better to introduce an explicit attribute to switch between package and non-package mode (cf #1132 (comment)).

[adriangb]

does it make sense to build a distribution without any package/module or do you only use packages = [] if you never intend to build an sdist/wheel?

I don't think it makes sense to build a distribution without packages, although I suppose it shouldn't be disallowed since it's a technically valid distribution and metapackages are a thing. But that's not my use case so I don't want to opine too much. It does make a lot of sense to want to install a meta project with no source code of its own. So at the very least, this error should either be a warning or only apply to building a distribution.

In the latter case, I think it's probably better to introduce an explicit attribute to switch between package and non-package mode (cf #1132 (comment)).

Perhaps. But my stance is going to be that this was a breaking change on a non major version release (breaking not only the existing behavior but also the promises made by the documentation, even if they were ambiguous) and that the change itself is questionable in the first place so it should be at least partially reverted (maybe to only apply to building distributions).

[silverwind]

this error should either be a warning

It is a warning as exit code is 0, but I agree the text output looks deceptively like an error.

[adriangb]

Also it seems to me like the goal was to prevent users from shooting themselves in the foot if they named their source code folder wrong or similar. Even if we made an exception for packages = [] the benefit of this change would be preserved.

[radoering]

this was a breaking change

I don't consider printing a warning a breaking change. (Every change is a breaking change for someone. If we move on this level, every new version is a major version.)

Also it seems to me like the goal was to prevent users from shooting themselves in the foot if they named their source code folder wrong or similar.

That and in more complicated cases, editable builds can just fail and without this change you will just notice that something does not work in your environment although poetry install "did not fail".

Even if we made an exception for packages = [] the benefit of this change would be preserved.

I agree that we can make an exception for packages = [] and clarify the docs (although this will be a breaking change for someone even if the current behavior can be interpreted as a bug). 😉

[adriangb]

I thought it was an error. I agree a warning is not a breaking change, but still a confusing annoyance.

[nikobockerman]

This warning is printed in red so it definitely looks like an error. I also got confused and thought it was an actual error.

[tueda]

After getting there and reading this issue, I understand this error-ish warning that contains the format <error>{e}</error> (leading to the red text) is actually not an error. Then the question is how the user can distinguish red text for a warning from that for an error. Maybe simply printing the word warning would be helpful.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.