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

Path dependency files not installed when listed in .gitignore #6624

Closed
4 tasks done
vwxyzjn opened this issue Sep 26, 2022 · 8 comments
Closed
4 tasks done

Path dependency files not installed when listed in .gitignore #6624

vwxyzjn opened this issue Sep 26, 2022 · 8 comments
Labels
status/duplicate Duplicate issues

Comments

@vwxyzjn
Copy link

vwxyzjn commented Sep 26, 2022

  • Poetry version: Poetry (version 1.2.1)
  • Python version:
Poetry
Version: 1.2.1
Python:  3.9.5

Virtualenv
Python:         3.9.5
Implementation: CPython
Path:           /home/costa/.cache/pypoetry/virtualenvs/poetry12bug-cjZNteOe-py3.9
Executable:     /home/costa/.cache/pypoetry/virtualenvs/poetry12bug-cjZNteOe-py3.9/bin/python
Valid:          True

System
Platform:   linux
OS:         posix
Python:     3.9.5
Path:       /home/costa/.pyenv/versions/3.9.5
Executable: /home/costa/.pyenv/versions/3.9.5/bin/python3.9
  • OS version and name:
             /////////////                costa@pop-os 
         /////////////////////            ------------ 
      ///////*767////////////////         OS: Pop!_OS 21.10 x86_64 
    //////7676767676*//////////////       Kernel: 5.17.5-76051705-generic 
   /////76767//7676767//////////////      Uptime: 45 days, 9 hours, 57 mins 
  /////767676///*76767///////////////     Packages: 3025 (dpkg), 6 (flatpak), 2 (snap) 
 ///////767676///76767.///7676*///////    Shell: zsh 5.8 
/////////767676//76767///767676////////   Terminal: node 
//////////76767676767////76767/////////   CPU: AMD Ryzen 9 3900X (24) @ 3.800GHz 
///////////76767676//////7676//////////   GPU: NVIDIA GeForce RTX 3060 Ti 
////////////,7676,///////767///////////   GPU: NVIDIA GeForce RTX 3060 Ti 
/////////////*7676///////76////////////   Memory: 7434MiB / 64237MiB 
///////////////7676////////////////////
 ///////////////7676///767////////////                            
  //////////////////////'////////////                             
   //////.7676767676767676767,//////
    /////767676767676767676767/////
      ///////////////////////////
         /////////////////////
             /////////////
  • I am on the latest stable Poetry version, installed using a recommended method.
  • I have searched the issues of this repo and believe that this is not a duplicate.
  • I have consulted the FAQ and blog for any relevant entries or release notes.
  • If an exception occurs when executing a command, I executed it again in debug mode (-vvv option) and have included the output below.

Issue

When installing a sub-project via relative path, the sub-project is not importable when the its path is in the .gitignore. See a demo below and a minimal reproduction here.

asciicast

@vwxyzjn vwxyzjn added kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Sep 26, 2022
@neersighted
Copy link
Member

I suspect this is an artifact of our .gitignore support excluding files from builds. It's somewhat surprising to me that these two parts of the code base interact, but I don't think it's necessarily wrong -- it looks like Poetry is just ignoring files earlier than the build step without documenting that behavior explicitly.

We honestly could go either way on this -- it could just be a docs fix to codify this behavior. Could you please explain why it might be desirable to install Python code from a path you have explicitly excluded from your repo? Typically relative parent paths are used, and they're generally preferable for this as it makes it clear they're not under the aegis of source control/missing files from a local clone.

@vwxyzjn
Copy link
Author

vwxyzjn commented Sep 26, 2022

Thanks for the prompt response. This is actually a pretty strange use case. NVIDIA's isaacgym is a python package that is not on PyPi (see isaac-sim/IsaacGymEnvs#23) , so we have to manually download it... But we couldn't keep it in source control because of the license agreement, so we have to manually put it in the gitingore.

Previously we were installing via the -E option, which works fine.. However, lately, we are migrating to poetry 1.2 (see vwxyzjn/cleanrl#271) and we are trying to use dependency groups, so this is where the bug happened.

We can keep the -E option specifically for this dependency, but not sure if this is the desired behavior for groups, too.

@neersighted neersighted changed the title Subproject relative path installation failed when it is in .gitignore Path dependency files not installed when listed in .gitignore Sep 26, 2022
@dimbleby
Copy link
Contributor

Your attempt to include looks wrong, I think you want something like

diff --git a/test/inner_project/pyproject.toml b/test/inner_project/pyproject.toml
index 2c6411b..c586da4 100644
--- a/test/inner_project/pyproject.toml
+++ b/test/inner_project/pyproject.toml
@@ -4,10 +4,10 @@ version = "0.1.0"
 description = ""
 authors = ["Costa Huang <costa.huang@outlook.com>"]
 readme = "README.md"
-packages = [{include = "inner_project"}]
+include = ["inner_project/*"]

@dimbleby
Copy link
Contributor

Also the path dependency and nested project structure is unnecessary complication for the repro, you can just poetry build the inner project and see whether the dist contains the code or not

@dimbleby
Copy link
Contributor

I was going to suggest that all there is to do here is maybe improve the docs, but actually https://python-poetry.org/docs/pyproject#include-and-exclude looks pretty reasonable - not sure how you decided to do something quite different?

If you think the docs can be improved then MR welcome, I don't think there's a bug though.

@neersighted
Copy link
Member

Ah, now I understand the interaction. The inner Poetry instance is recursing up and finding itself in the .gitignore, and thus ignoring all files during the build. This is a duplicate and specialized variant of #5547 -- with that understood, I will close it. I would suggest using @dimbleby's fix which will work as advertised -- and if you require support/have more questions, ask that we migrate this issue to a discussion, or join us on Discord.

@neersighted neersighted closed this as not planned Won't fix, can't repro, duplicate, stale Sep 26, 2022
@neersighted neersighted added status/duplicate Duplicate issues and removed kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Sep 26, 2022
@vwxyzjn
Copy link
Author

vwxyzjn commented Sep 27, 2022

Thank you! I tried that include = [“inner_project/**/*”] and it worked!

Copy link

github-actions bot commented Mar 1, 2024

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status/duplicate Duplicate issues
Projects
None yet
Development

No branches or pull requests

3 participants