tpaviot / pythonocc-core

Python package for 3D geometry CAD/BIM/CAM
GNU Lesser General Public License v3.0
1.39k stars 380 forks source link

Fix windows dll path #1384

Closed tpaviot closed 2 weeks ago

tpaviot commented 2 weeks ago

This PR is a follow up for #1347

@farleyrunkel @Andrej730 @Bill-XU can you please review this PR I cannot test the fix.

Summary by Sourcery

Fix the DLL path issue on Windows by adding a function to initialize OCCT libraries using the OCCT_ESSENTIALS_PATH environment variable. Update the INSTALL.md to provide a detailed build guide for Linux and Windows, including prerequisites, system requirements, and troubleshooting.

Bug Fixes:

Documentation:

sourcery-ai[bot] commented 2 weeks ago

Reviewer's Guide by Sourcery

This PR implements a fix for Windows DLL loading by adding a new initialization mechanism that adds OpenCascade DLL directories to the Windows DLL search path. The changes also include a major update to the installation documentation with comprehensive guides for both Linux and Windows platforms.

Sequence diagram for Windows DLL initialization

sequenceDiagram
    actor User
    participant Application
    participant OS
    participant OpenCascade

    User->>Application: Start Application
    Application->>OS: Check platform
    alt Windows
        OS->>Application: Platform is Windows
        Application->>OS: Check OCCT_ESSENTIALS_PATH
        alt OCCT_ESSENTIALS_PATH set
            OS->>Application: OCCT_ESSENTIALS_PATH is set
            Application->>OpenCascade: Initialize OCCT libraries
            loop For each DLL in OCCT_ESSENTIALS_PATH
                OpenCascade->>OS: Add DLL directory to search path
            end
        else OCCT_ESSENTIALS_PATH not set
            OS->>Application: OCCT_ESSENTIALS_PATH not set
            Application->>User: Raise AssertionError
        end
    else Not Windows
        OS->>Application: Platform is not Windows
    end

File-Level Changes

Change Details Files
Added Windows DLL path initialization mechanism
  • Created initialize_occt_libraries function to add OpenCascade DLL directories to the search path
  • Added automatic DLL path initialization when running on Windows
  • Added validation of OCCT_ESSENTIALS_PATH environment variable
src/PkgBase/__init__.py
Comprehensive documentation update for installation process
  • Added detailed Windows installation guide
  • Restructured Linux installation guide with clear sections
  • Added troubleshooting section for common issues
  • Added table of contents for better navigation
  • Updated system requirements and prerequisites for both platforms
INSTALL.md

Tips and commands #### Interacting with Sourcery - **Trigger a new review:** Comment `@sourcery-ai review` on the pull request. - **Continue discussions:** Reply directly to Sourcery's review comments. - **Generate a GitHub issue from a review comment:** Ask Sourcery to create an issue from a review comment by replying to it. - **Generate a pull request title:** Write `@sourcery-ai` anywhere in the pull request title to generate a title at any time. - **Generate a pull request summary:** Write `@sourcery-ai summary` anywhere in the pull request body to generate a PR summary at any time. You can also use this command to specify where the summary should be inserted. #### Customizing Your Experience Access your [dashboard](https://app.sourcery.ai) to: - Enable or disable review features such as the Sourcery-generated pull request summary, the reviewer's guide, and others. - Change the review language. - Add, remove or edit custom review instructions. - Adjust other review settings. #### Getting Help - [Contact our support team](mailto:support@sourcery.ai) for questions or feedback. - Visit our [documentation](https://docs.sourcery.ai) for detailed guides and information. - Keep in touch with the Sourcery team by following us on [X/Twitter](https://x.com/SourceryAI), [LinkedIn](https://www.linkedin.com/company/sourcery-ai/) or [GitHub](https://github.com/sourcery-ai).
tpaviot commented 2 weeks ago

@Bill-XU thank you for your comments. For anything related to documentation on building occt/pythonocc for windows, you can open a new issue/pr.

Andrej730 commented 2 weeks ago

I have a suggestion to include setup of OCCT_ESSENTIALS_PATH in cmake on windows, so it will be set up automatically during the pythonocc installation.

tpaviot commented 2 weeks ago

@Andrej730 You mean the OCCT_ESSENTIALS_PATH would be hard coded after the build is completed?

Andrej730 commented 2 weeks ago

@Andrej730 You mean the OCCT_ESSENTIALS_PATH would be hard coded after the build is completed?

Yeah, I think, this is what most users would expect if build succeeded and Python package is installed - Python package to work without setting up additional environment variable manually.

tpaviot commented 2 weeks ago

ok that makes sense I will add this feature

tpaviot commented 2 weeks ago

I'm sorry @Bill-XU I don't get the point between the _ROOT, _PATHS, "root of opencascade" etc. this is specific windows stuff. You can let the OCCT_ESSENTIALS_ROOT env var point wherever needed. If you need a different behavior please submit a patch.

Bill-XU commented 2 weeks ago

I'm sorry @Bill-XU I don't get the point between the _ROOT, _PATHS, "root of opencascade" etc. this is specific windows stuff. You can let the OCCT_ESSENTIALS_ROOT env var point wherever needed. If you need a different behavior please submit a patch.

@tpaviot Err... my apologize for not saying it clearly. The point is that, the value of the env var should not be "%CASROOT%\win64\vc14\bin", inside which there are only OpenCascade DLLs. As @Andrej730 mentioned in #1347, it is necessary to load not only opencascade DLLs but also 3rd party libraries.

tpaviot commented 2 weeks ago

@Andrej730 I tweaked CmakeList.txt so it becomes possible to define OCCT_ESSTNEIALS_ROOT at build time and hard code the related path to config.py. It can however still be defined at run time. Can you please test and report any issue?

@Bill-XU I think the INSTALL.md is now more accurate, can you please review.

tpaviot commented 2 weeks ago

BTW @Andrej730 I remember you discussed a pythonocc issue, related to shape serialization and version upgrade, but I can't find it anymore, can you please point me to the right place?

Andrej730 commented 2 weeks ago

I remember you discussed a pythonocc issue, related to shape serialization and version upgrade, but I can't find it anymore, can you please point me to the right place?

@tpaviot I believe this is the one - https://github.com/tpaviot/pythonocc-core/issues/1350#issuecomment-2227910142