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

Install extensions as console_scripts #173

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

twmr
Copy link

@twmr twmr commented Mar 5, 2022

Since a setup.py file exists in the src tree, it should also be used for the installation, which is currently not the case if one follows the instructions in the README. The README says that the requires files should be copied into the inkscape extensions directory. IMO the files in the silhouette directory should be properly installed and executables of the two extensions should be created, which should be used in the inx files.

This is done in this PR. Furthermore the pytest related code from setup.py was removed, because
the test subcommand of setup.py is deprecated.

@twmr twmr force-pushed the setupimp branch 4 times, most recently from c962733 to 02ca76f Compare March 5, 2022 21:07
@t0b3
Copy link
Collaborator

t0b3 commented Mar 21, 2022

While the current default installation method is to install the files as a Inkscape Extension...
this PR is designed to IIUC:

  1. modernize setup.py (drop test, etc.)
  2. change install to console_script for standalone use
  3. adapt instructions and installation scripts for new installation method

I'd like to concentrate this PR to changes [2] and [3].

__

May I ask: will the changes [part 2]

  • work to provide the extension in one bundle to consume via inkscape extension manager (see inkscape menu -> extensions -> manage -> install and https://inkscape.org/develop/extensions/)?
  • or would they involve to install two separate parts (inkscape extension, python package) and keep them in sync subsequently?
    image

It would be great to know how these changes support solving some nasty snap related path issues for inkscape in the future i.e. via deploying the extension for the extension manager... Thanks for clarification.
__

PS: In case we choose to abandon the current dual installation possibilities and switch to solely pip install method then we need to ensure it will apply everywhere in the right context (system vs. user: i.e. pip install --user) and for packaging for all platforms (linux, mac, win) and adapt installation instructions as well. This would need to consider the build-deb workflow, the relevant parts of the installation related files and workflows as well as more chapters of Readme.


install -m 755 -t $(DEST) *silhouette*.py

extension_files = ["sendto_silhouette.inx", "sendto_silhouette.py",

cp -a ../silhouette $name/

Install inkscape-silhouette
* https://github.com/fablabnbg/inkscape-silhouette/releases
Scroll down to Downloads and click on the *.deb file.

sudo make install # OR: make install-local # latter installs only for this user

etc.
...

@twmr
Copy link
Author

twmr commented Mar 21, 2022

@t0b3 thx a lot for your feedback! You are right that my PR basically consists of 3 parts (setup.py, console-scripts, installation scripts).

I'd like to concentrate this PR to changes [2] and [3].

Fine by me. I'll remove the setup.py changes and create a separate PR for them once parts 2 and 3 are merged.

will the changes [part 2] work to provide the extension in one bundle to consume via inkscape extension manager?

I think it is possible to extend setup.py to install the inx file to the desired location e.g. into the home directory of the user (~/.config/inkscape/extensions). But I don't know what is required to create a bundle that can be installed via the inkscape extension manager.

I just had a look at two extensions that are part of the inkscape-extension-store:

https://gitlab.com/inkscape/extras/inkscape-import-clipart
https://github.com/raniaamina/inkporter/tree/inkporter-gui

but none of them use setup.py to install the extension. I'll look further - maybe I can find an extension that has already solved the installation/deployment problem.

@twmr
Copy link
Author

twmr commented Mar 21, 2022

In order to simplify and generalize the installation instructions, we could bundle all files (inkscape extension + python library) into a python pkg, which can be installed using pip. Part of this package should be a new console script that copies the extension related files to a desired destination location. So, the user would have to install the pkg using pip and then run this inkscape-extension-copy script.

twmr added 2 commits April 3, 2022 16:59
The two scripts (silhouette_multi.py and sendto_silhouette.py) are now
part of the `silhouette` python pkg and the executables are created and
installed by setuptools (see the defined console scripts in setup.py).

Furthermore the pytest related code from setup.py was removed, because
the `test` subcommand of `setup.py` is deprecated.
They will be done in a separate PR.
@twmr
Copy link
Author

twmr commented Apr 3, 2022

@t0b3 I've removed the setup.py changes from this PR as you suggested. In a separate PR I'll create a new console script that allows the users copy the inx files into the desired extension directory. This would allows us to simplify the installation instructions. WDYT?

@t0b3
Copy link
Collaborator

t0b3 commented Apr 3, 2022

Which use case will this PR contribute to? Is this PR suggesting to install the inkscape extension via pip -> script -> inkscape extension?

If yes

  • how would you like to avoid this becoming a new point of failure to keep the versions and paths of INX and PY files in sync in all those installations out there...
  • how would this affect the update process? is manual intervention needed? ...
  • how would this affect selection of installation paths? is user knowledge of paths needed? ...

and for which OS and which steps do you feel do the installation instructions sound complicated?

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