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

Improvements to ease packaging work on Linux #156

Merged
merged 7 commits into from
Dec 12, 2024
Merged

Conversation

frenchwr
Copy link
Contributor

@frenchwr frenchwr commented Dec 6, 2024

  • Remove typing from setup.py as this ships with the standard library since Python 3.5, and installing it as a separate package can cause conflicts on Ubuntu
  • Update import method in model_setup.py to allow this script to be run from anywhere
  • Commit executable permissions to plugin files in this repo and ship these plugins as data_files installed at gimp-plugins/. Unfortunately without adding these to data_files the files do not retain their executable permissions. The absolute path to the gimp-plugins/ file is system/environment-dependent. For example, in a virtualenv this directory will be in the root directory of the virtualenv. On Ubuntu 24.04 pip-installing gimpopenvino system-wide puts the directory at /usr/local/gimp-plugins.

I tested these changes (by running some simple tests for each plugin) within a GIMP snap I have built locally running on Ubuntu 24.04, but I have NOT tested on Windows or even the install.sh script on Linux.

@frenchwr
Copy link
Contributor Author

frenchwr commented Dec 7, 2024

I realized that part of the changes I made in install.sh will not work because gimpopenvino itself is not actually installed into the virtualenv...I'll make some updates on Monday.

@frenchwr
Copy link
Contributor Author

frenchwr commented Dec 9, 2024

Ok this is ready to be tested.

I ended up reverting a few things since Friday and my most recent commit installs the plugins from the source repo rather than from the virtualenv, that way the plugin .py files retain their executable permission (they lose this permission when installed into the virtualenv) and we don't need to update the file permissions from complete_install.py (since containerized packaging tools may not allow updates like this).

install.sh Outdated
@@ -49,7 +49,7 @@ deactivate
echo "Installing plugin in $HOME/.config/GIMP/2.99/plug-ins"
for d in openvino_utils semseg_ov stable_diffusion_ov superresolution_ov; do
mkdir -p "$HOME/.config/GIMP/2.99/plug-ins/$d"
rsync -a gimpenv3/lib/python*/site-packages/gimpopenvino/plugins/"$d" "$HOME/.config/GIMP/2.99/plug-ins/."
rsync -a "$script_dir/gimpopenvino/plugins/$d" "$HOME/.config/GIMP/2.99/plug-ins/."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue with this line is that it will miss the gimp_openvino_config.json file copy to the default config_dir_path in the case that an Environment variable is not used.

The goal here is to copy from the $script_dir because of the executable permissions change needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah I missed that. Yes, the goal is to avoid running chmod +x on the plugin files from complete_install.py because the library is already installed at this point...so this causes some problems for a containerized packaging format like snaps use. It's not a major issue, just a nice-to-have.

I could just settle for wrapping the chmod's with a try-except. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it is not a major issue, we can bin it for now and wrap the chmod commands with try-except. i think the real solution is to re-think how the config file gets created the first time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I just reverted all the chmod -related changes for the plugins. I didn't even bother with a try-except because suprocess.call() does not throw an exception even if the shell command fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also let me know if you'd like me to clean up the commits. I often use the squash merge method from GitHub but if you prefer I clean up the commits locally just let me know. :)

Copy link
Contributor

@gblong1 gblong1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Tested on Windows and Linux.

@gblong1 gblong1 merged commit 7a84f15 into intel:snap Dec 12, 2024
gblong1 added a commit that referenced this pull request Jan 11, 2025
* Set config and model paths dynamically from environment vars

Signed-off-by: Will French <[email protected]>

* Centralizing ENV VARs into tools_utils.

* Fixing default path.

* fixing test for python executable logic

* Adding threading to SR to avoid not responding messages.

* removing debug statement

* Improvements to ease packaging work on Linux (#156)

* Allow model_setup to be run from any directory

* Store plugin scripts with executable permissions

* Remove typing as it ships with stdlib since python 3.5

* Install plugin files with executable permissions preserved

* Revert data_files addition to avoid complexity

* Install plugins from source repo instead of venv

* Revert permissions changes to test failure inside snap

* Initial update of SR to GIMP 3.0.

* Updating Save Image to work with GIMP 3.0

* Removing unneeded comments.

* Initial update of Segmentation to GIMP 3.0

* Removing uneeded files.

* Updating install path

* Moving to latest OpenVINO - 2024.6.0

* removing unneeded log.info calls.

* Updating proc id for GIMP 3.0

* Updating plugin version number

* Initial update of SD to GIMP 3.0

* Updating option caching in SD.

* removed files that are no longer used.

* Implemented SDOptionCache to address issues with options being saved run-to-run.

* Removed debug prints, unneeded syspath extention

* remove commented syspath line

* Adding version support for plugins.

* updated install flow. removed need to call complete_install separately.

* Hiding console for SR and SS. SD is still WIP

* removed .json config, as it should be created from scratch everytime.

* restored alignment of OV logo.

* Updated NPU Arch logic to support next gen NPUs

    * Updated NPU Arch logic to support next gen NPUs

    * update ARCH_NEXT and tweak logic

    * Fixed NPU Arch logic to work correctly.

    * Add check for dNPU to avoid NPU turbo enabling exception for those devices

* workaround for 3700 series being reported incorrectly in OV2024.5+

* restore running complete_install post setup

* update tools_utils import

* subprocess call is diff for Linux vs Windows

* Removing import of install_utils.

It relies on importing openvino, which was causing issues in Linux for some reason.
For now, we are have the get_plugin_version code duplicated here. Need to find a way
to remove it in the future.

* Longer timeout needed for some systems.

* Updating title bar to show just the plugin version.

* updating process name to work with Linux

* Adding linux support for BIOS discovery

* Really fixing issue #158 this time.

* Updating linux build docs

* fixing issue with ENV var installation and commandline model installs.

* Updating download links for GIMP3 RC2

* Updating figure for GIMP3 RC2

* Updating setup complete figure for GIMP3 RC2

* Updates for Docs and Figures for GIMP 3.0

---------
Signed-off-by: Garth Long <[email protected]>
Signed-off-by: Will French <[email protected]>
Co-authored-by: Will French <[email protected]>
Co-authored-by: Ryan Metcalfe <[email protected]>
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.

2 participants