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

Setup changes #449

Merged
merged 6 commits into from May 22, 2019
Merged

Setup changes #449

merged 6 commits into from May 22, 2019

Conversation

bcail
Copy link
Contributor

@bcail bcail commented Apr 25, 2019

Remove initialization code from setup.py - setup.py just installs the python package. Add a user_commands.py module for users to run setup/init commands after install, if desired.

@bcail bcail marked this pull request as ready for review April 25, 2019 18:46
@minusdavid
Copy link

Does the change work when installing with pip?

From a packager perspective, this isn't quite good enough. Could you add a BASE_DIR or PREFIX for each of the files created by the "create_default_files_and_directories" function? That way, a packager could specify a package-specific buildroot and then move the files to the vendor specific place they need to go.

@minusdavid
Copy link

It feels like my suggestion is re-inventing the wheel... so surely there must be other Python web apps that get packaged which have solved this one.

That said, it looks like other people have grappled with this question:
https://wiki.debian.org/DjangoPackagingDraft

This one is very old but I think was the basis of the Koha Debian package:
https://people.debian.org/~neilm/webapps-policy/

@minusdavid
Copy link

Seems like Python admits that it's a difficult problem as well:
https://packaging.python.org/overview/

Looking at https://packages.debian.org/stretch/fail2ban, it looks like it uses data_files for /etc and /var files.

At fail2ban/fail2ban#1536, they also refer to using --root for deployments.

According to some comments at https://ubuntuforums.org/showthread.php?t=1713458, Debian package scripts call the following under the hood:

python setup.py build
python setup.py install --root=debian/package-name/usr --install-layout=deb

The following links speak to that more:
https://www.debian.org/doc/packaging-manuals/python-policy/packaging_tools.html#dh-python
https://manpages.debian.org/testing/python/dh_python2.1.en.html

@minusdavid
Copy link

The more I look at this... the more I'd have to sit down and spend some time trying out package builds again. Rather than just Googling on a Saturday night... heh.

@bcail
Copy link
Contributor Author

bcail commented May 2, 2019

@minusdavid thanks for looking at this. If you want to customize where files are installed, one way to do that with this PR is to customize and pass a python config object into the user_commands.create_default_files_and_directories function. Take a look at the tests/user_commands_t.py module, where I set the function to install the files into a temp directory.

Does that help with creating a package at all? If not, could this PR be a first step, and then we add another option if needed?

@minusdavid
Copy link

@bcail Thanks for doing the work!

But no I don't think that would help with creating a package.

But yeah I think this PR would be a first step and then something else could be done to aid with packaging. I don't have the time to invest in this anymore, so I don't want to hold anyone up either.

@bcail
Copy link
Contributor Author

bcail commented May 16, 2019

ok, thanks @minusdavid. @alexwlchan any thoughts on this?

Copy link
Contributor

@alexwlchan alexwlchan left a comment

Choose a reason for hiding this comment

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

Getting this code out of setup.py seems a good idea to me.

One suggestion for improving code reuse but otherwise nothing big from me; I don’t know a huge amount about packaging.

loris/user_commands.py Outdated Show resolved Hide resolved
@bcail bcail merged commit 9069177 into development May 22, 2019
@bcail bcail deleted the setup branch October 2, 2019 14:42
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

3 participants