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

Port all setup.py files to setuptools #132

Closed
wants to merge 2 commits into from
Closed

Conversation

tiran
Copy link
Member

@tiran tiran commented Oct 4, 2016

PREVIEW, please don't merge yet.

This is my take on FreeIPA's setup.py files. I have moved all common code and constants into ipasetup.py.in and converted all setup.py.in to setup.py. Setup now uses setuptools instead of distutils. I had to move ipa.1 man page to client/man because setuptools does no longer support data_files.

Signed-off-by: Christian Heimes cheimes@redhat.com

download_url="http://www.freeipa.org/page/Downloads",
platforms=["Linux", "Solaris", "Unix"],
classifiers=[
"Development Status :: 4 - Beta",
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can change this to anything else than beta :D

Copy link
Contributor

Choose a reason for hiding this comment

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

hub am #132
Applying: Draft for a new setup.py
.git/rebase-apply/patch:316: new blank line at EOF.
+
warning: 1 line adds whitespace errors.

Also please fix this

@MartinBasti
Copy link
Contributor

This WIP works for me, I like that we get rid of setup.py.in files.

I'm looking forward to final version

Please fix PEP8 reported error and my inline comments

@tiran
Copy link
Member Author

tiran commented Oct 6, 2016

@mbasti-rh I have removed more hacks and made each setup.py even simpler.

@@ -41,6 +41,9 @@ freeipa2-dev-doc
/dist/
/RELEASE
/rpmbuild/
# Build
ipasetup.py
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is ipasetup.py ignored?

Copy link
Member Author

Choose a reason for hiding this comment

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

Makefile autogenerates the file from ipasetup.py.in.

Copy link
Contributor

Choose a reason for hiding this comment

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

slaps forehead Right.

for directory in ipaclient ipalib ipaplatform ipapython ipatests; do \
pushd $${directory} ; \
cp ../ipasetup.py . ; \
$(PYTHON) setup.py egg_info ; \
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO it would be better to run setup.py with PYTHONPATH set properly, rather than copy ipasetup.py all the time.

Copy link
Member Author

Choose a reason for hiding this comment

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

With a copy of ipasetup.py there are no build dependencies between the packages. I can simple chdir into a directory and run python setup.py install.

Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively, if sys.path was updated appropriately in each setup.py, we would get rid of copying and allow chdir & run setup.py.

@@ -10,7 +10,8 @@ man1_MANS = \
ipa-client-install.1 \
ipa-client-automount.1 \
ipa-certupdate.1 \
ipa-join.1
ipa-join.1 \
ipa.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Either put the ipa.1 packaging change in a separate commit, or don't do it at all (there must be some way to package custom file with setuptools, right?)

Copy link
Member Author

Choose a reason for hiding this comment

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

No, setuptools no longer supports non-package data. It can no longer install files to PREFIX/share, PREFIX/man or any other directory except for scripts (PREFIX/share) and package data.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm going to move the change to a separate commit.

@MartinBasti
Copy link
Contributor

I cannot reinstall packages using your commits

dnf reinstall <IPA*>.rpm
...
Error: Transaction check error:
  file /usr/lib/python2.7/site-packages/ipalib-4.4.90-py2.7.egg-info from install of python2-ipalib-4.4.90-0.fc24.noarch conflicts with file from package python2-ipalib-4.4.90-0.fc24.noarch
  file /usr/lib/python2.7/site-packages/ipaplatform-4.4.90-py2.7.egg-info from install of python2-ipalib-4.4.90-0.fc24.noarch conflicts with file from package python2-ipalib-4.4.90-0.fc24.noarch
  file /usr/lib/python2.7/site-packages/ipapython-4.4.90-py2.7.egg-info from install of python2-ipalib-4.4.90-0.fc24.noarch conflicts with file from package python2-ipalib-4.4.90-0.fc24.noarch
  file /usr/lib/python2.7/site-packages/ipaclient-4.4.90-py2.7.egg-info from install of python2-ipaclient-4.4.90-0.fc24.noarch conflicts with file from package python2-ipaclient-4.4.90-0.fc24.noarch
  file /usr/lib/python3.5/site-packages/ipalib-4.4.90-py3.5.egg-info from install of python3-ipalib-4.4.90-0.fc24.noarch conflicts with file from package python3-ipalib-4.4.90-0.fc24.noarch
  file /usr/lib/python3.5/site-packages/ipaplatform-4.4.90-py3.5.egg-info from install of python3-ipalib-4.4.90-0.fc24.noarch conflicts with file from package python3-ipalib-4.4.90-0.fc24.noarch
  file /usr/lib/python3.5/site-packages/ipapython-4.4.90-py3.5.egg-info from install of python3-ipalib-4.4.90-0.fc24.noarch conflicts with file from package python3-ipalib-4.4.90-0.fc24.noarch
  file /usr/lib/python3.5/site-packages/ipaclient-4.4.90-py3.5.egg-info from install of python3-ipaclient-4.4.90-0.fc24.noarch conflicts with file from package python3-ipaclient-4.4.90-0.fc24.noarch
  file /usr/lib/python3.5/site-packages/ipatests-4.4.90-py3.5.egg-info from install of python3-ipatests-4.4.90-0.fc24.noarch conflicts with file from package python3-ipatests-4.4.90-0.fc24.noarch
  file /usr/lib/python2.7/site-packages/ipatests-4.4.90-py2.7.egg-info from install of python2-ipatests-4.4.90-0.fc24.noarch conflicts with file from package python2-ipatests-4.4.90-0.fc24.noarch

@tiran
Copy link
Member Author

tiran commented Oct 18, 2016

@mbasti-rh I can't reproduce your problem. dnf reinstall works for me. I tested both upgrades from 4.4.2 and reinstall of my RPMs.

@@ -1,2 +1 @@
include COPYING TODO lite-server.py
include tests/*/*.py
include COPYING lite-server.py
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this manifest file used? Can't we just drop it? I think it at least should not include lite-server.py.

@tiran tiran force-pushed the new_setup branch 6 times, most recently from 6a6cdf9 to 4c59f8a Compare October 20, 2016 09:54
The file ipapython/ipa.conf is no longer used and not installed.

Signed-off-by: Christian Heimes <cheimes@redhat.com>
@tiran tiran mentioned this pull request Oct 20, 2016
@tiran tiran changed the title Draft for a new setup.py (WIP) Port all setup.py files to setuptools Oct 20, 2016
@tiran
Copy link
Member Author

tiran commented Oct 20, 2016

The PR is no longer provisional and ready-to-merge.

version=ipalib.__version__,
license='GPLv3+',
url='http://freeipa.org/',
scripts=['ipa'],
Copy link
Contributor

Choose a reason for hiding this comment

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

The ipa script is in fact a client script, so I guess it shouldn't be packaged with ipaserver.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should the script be installed by ipalib or ipaclient? It's a client script but the actual code is in ipalib.

@HonzaCholasta
Copy link
Contributor

LGTM, but shouldn't we also move /setup.py to /ipaserver/setup.py?

All setup.py files are now using setuptools through a common file
ipasetup.py. The file is auto-generated and contain all common
settings.

Signed-off-by: Christian Heimes <cheimes@redhat.com>
@tiran
Copy link
Member Author

tiran commented Oct 20, 2016

I have moved /setup.py into /ipaserver/, fixed some make distclean targets (they were still removing setup.py) and made /ipaclient/setup.u install the ipa script. make rpms is passing.

@pspacek pspacek self-assigned this Oct 20, 2016
@pspacek
Copy link
Contributor

pspacek commented Oct 20, 2016

Jenkins does not complain. ACK.

@pspacek pspacek added the ack Pull Request approved, can be merged label Oct 20, 2016
@pvoborni pvoborni added the pushed Pull Request has already been pushed label Oct 20, 2016
@pvoborni pvoborni closed this Oct 20, 2016
@tiran tiran deleted the new_setup branch October 24, 2016 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
5 participants