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

Various fixes for pip file discovery #7682

Closed
wants to merge 2 commits into from
Closed

Conversation

berniev
Copy link
Contributor

@berniev berniev commented Nov 2, 2022

Thank you for creating a pull request to contribute to FreeCAD! To ease integration, we ask you to conform to the following items. Pull requests which don't satisfy all the items below might be rejected. If you are in doubt with any of the items below, don't hesitate to ask for help in the FreeCAD forum!

  • Your pull request is confined strictly to a single module. That is, all the files changed by your pull request are either in App, Base, Gui or one of the Mod subfolders. If you need to make changes in several locations, make several pull requests and wait for the first one to be merged before submitting the next ones
  • In case your pull request does more than just fixing small bugs, make sure you discussed your ideas with other developers on the FreeCAD forum
  • Your branch is rebased on latest master git pull --rebase upstream master
  • All FreeCAD unit tests are confirmed to pass by running ./bin/FreeCAD --run-test 0
  • All commit messages are well-written ex: Fixes typo in Draft Move command text
  • Your pull request is well written and has a good description, and its title starts with the module name, ex: Draft: Fixed typos
  • Commit messages include issue #<id> or fixes #<id> where <id> is the issue ID number from our Issues database in case a particular commit solves or is related to an existing issue. Ex: Draft: fix typos - fixes #4805

And please remember to update the Wiki with the features added or changed once this PR is merged.
Note: If you don't have wiki access, then please mention your contribution on the 1.0 Changelog Forum Thread.


Corrects multiple issues causing "include files not found" warning for pip-based pyside and shiboken

@freecadci
Copy link

pipeline status for feature branch PR_7682. Pipeline 683110211 was triggered at 4b753ff. All CI branches and pipelines.

@chennes
Copy link
Member

chennes commented Nov 2, 2022

Thanks for the fix -- the code looks good. Can you amend the commit message to describe what it does? You can see an example of what our commit messages usually look like using git log --oneline (or git log, if you want the whole thing).

@chennes
Copy link
Member

chennes commented Nov 3, 2022

I see you've edited the PR description -- to be clear, I am asking specifically that the commit be amended. The PR will get lost in the sands of time, but the commit is in the history forever. The easiest is to use git commit --amend (assuming this is the last commit in your branch), but you can also do an "interactive rebase" using git rebase -i which has many more options. You'll have to force-push the branch after making the edit there.

@berniev
Copy link
Contributor Author

berniev commented Nov 3, 2022

Despite getting that section of code to at least work, and avoid an error, there is still more to do (build error)

@berniev
Copy link
Contributor Author

berniev commented Nov 3, 2022

ninja: error: '/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/shiboken6/lib', needed by 'lib/libFreeCADGui.dylib', missing and no known rule to make it

@chennes
Copy link
Member

chennes commented Nov 3, 2022

Do you want to switch this PR to draft mode while you continue to work on it? Or would you prefer to merge this improvement, and work on fixing that bug separately?

@berniev
Copy link
Contributor Author

berniev commented Nov 3, 2022

Now that pip file discovery code works, it seems to expose an error in the underlying assumptions. As the original author, perhaps you could take a look?

@freecadci
Copy link

pipeline status for feature branch PR_7682. Pipeline 685210313 was triggered at d527d6f. All CI branches and pipelines.

@berniev
Copy link
Contributor Author

berniev commented Nov 18, 2022

Added some cleanups to make going forward easier

@freecadci
Copy link

pipeline status for feature branch PR_7682. Pipeline 698692425 was triggered at c29ef73. All CI branches and pipelines.

@freecadci
Copy link

pipeline status for feature branch PR_7682. Pipeline 698778051 was triggered at 66d5273. All CI branches and pipelines.

@berniev
Copy link
Contributor Author

berniev commented Nov 21, 2022

@chennes Your code specifies a /lib directory as _LIBRARY. My pip-installed PySide and Shiboken directories do not have such a lib directory. Assuming the original code was somehow tested (yes?) , then the question becomes why doesn't my installation have that directory?

@chennes
Copy link
Member

chennes commented Nov 21, 2022

Interesting -- in my Windows install of PySide6 there is a lib and include directory, but not in PySide2 (which doesn't matter because on Windows we ship with PySide2 as a cMake-findpackage-findable package). It looks to me like the libraries are just in the top-level directory on my system (e.g. not within lib at all). Is that the case on yours? (Homebrew Mac, I think?)

@berniev
Copy link
Contributor Author

berniev commented Nov 21, 2022

I don't understand most of that. We are talking pip-installed PySide6 and Shiboken6 on macOS. No system-installed, no Homebrew.
-- Found pip-installed Shiboken6 in /Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/shiboken6

-- Found PYSIDE6 tools: /usr/local/share/qt/libexec/uic, /usr/local/share/qt/libexec/rcc

These libraries you mention, what do they look like? .so? .dylib? Something else?

@chennes
Copy link
Member

chennes commented Nov 21, 2022

On my Mac I have libpyside6.abi3.6.4.dylib. On Windows I have pyside6.abi3.dll and pyside6.abi3.lib.

@berniev
Copy link
Contributor Author

berniev commented Nov 22, 2022

And for shiboken I have libshiboken6.abi3.6.3.dylib. I had tried setting the _LIBRARY to the "foundAt" directory but that triggers a whole bunch of cmake warnings. How to proceed? This is outside my comfort zone.

@chennes
Copy link
Member

chennes commented Nov 22, 2022

I guess we better reach out to our Python-integration experts -- it's possible the find_pip_package isn't needed at all. I was under the impression that it was better for us to link against those libraries, but we didn't make any effort to locate pip-installed PySide2 or Shiboken2. @wwmayer, Qt's documentation suggests installing using pip, so I thought it would be a good idea to try to find it in cMake, but it seems it's not as straightforward as I had hoped. Is this needed/worth pursuing?

@berniev
Copy link
Contributor Author

berniev commented Nov 22, 2022

Homebrew has been one drama after another despite lots of fiddling. Maybe Qt came to the same conclusion. Pip is very much worth pursuing. A bit of testing along the way might be an idea though.

@freecadci
Copy link

pipeline status for feature branch PR_7682. Pipeline 729551302 was triggered at 3c05045. All CI branches and pipelines.

@freecadci
Copy link

pipeline status for feature branch PR_7682. Pipeline 736154804 was triggered at 0f3d771. All CI branches and pipelines.

@berniev berniev marked this pull request as draft January 10, 2023 18:30
@berniev
Copy link
Contributor Author

berniev commented Jan 10, 2023

Given bad experiences with Homebrew I've opened a new can of worms locally (macOS) by download and install python from python.org and associated stuff eg pyside etc via pip.

Unfortunately find_package doesn't find pip-installed stuff (no cmake files), albeit trivial to discover them using python.

@chennes @wwmayer Could one or both of you please give me a brief shove in the right direction? I get that python can import python programs. But some have in their directory variously .so and/or .dylib files which I understand are linkable. What to do with these? I assume the linked versions are used somewhere and should be linked where possible? Also the interplay with include and library vriables.

@mosfet80
Copy link
Contributor

Itried this patch.
I installed shiboken6 and pyside6 through pip on ubuntu 23.10.

Before the patch this error was displayed:
FreeCAD/src/Gui/PythonWrapper.cpp:117:11: fatal error: basewrapper.h: No such file or directory
117 | # include <basewrapper.h>
| ^~~~~~~~~~~~~~~

After:
ninja: error: '/usr/local/lib/python3.11/dist-packages/shiboken6/lib', needed by 'lib/libFreeCADGui.so', missing and no known rule to make it

@berniev
Copy link
Contributor Author

berniev commented Aug 22, 2023

Yes, it is broken. The history is self-explanatory, I think.

@berniev berniev closed this by deleting the head repository May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants