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

Remove obsolete files setup_pypi.py and setup_standalone.py #1594

Closed
matthewfeickert opened this issue Jan 18, 2022 · 5 comments · Fixed by #1656
Closed

Remove obsolete files setup_pypi.py and setup_standalone.py #1594

matthewfeickert opened this issue Jan 18, 2022 · 5 comments · Fixed by #1656

Comments

@matthewfeickert
Copy link
Contributor

It seems that the files bindings/python/setup_pypi.py and bindings/python/setup_standalone.py are obsolete and could be deleted to avoid confusion and maintenance. This is motivated by noticing that bindings/python/setup_pypi.py contains Python 2 only code

print 'XRootD library dir: ', xrdlibdir
print 'XRootD include dir: ', xrdincdir
print 'Version: ', version

and so can't have been used for anything recently and has only had 4 changes since it was introduced in 2015 in e5b7572. Is there any reason to keep this around?

bindings/python/setup_standalone.py has only has 1 minor change to it since @nsmith- introduced it in a8c7110 in 2019. @nsmith- I'm not sure if you have a use case for this still, but I'm trying to introduce changes at the moment that modernize the Python installation, so I would hope these would be covered in them.

@nsmith-
Copy link
Contributor

nsmith- commented Jan 18, 2022

As in the original PR comment, the main use for setup_standalone is to be able to add a python binding when the library is already available systemwide. If there's a self-contained pip install xrootd in the future that will include the main library and successfully compile then this standalone setup is obviated.

@simonmichal
Copy link
Contributor

@matthewfeickert & @nsmith- : is there an action needed or can we just close this issue?

@matthewfeickert
Copy link
Contributor Author

matthewfeickert commented Mar 10, 2022

@matthewfeickert & @nsmith- : is there an action needed or can we just close this issue?

If there's a self-contained pip install xrootd in the future that will include the main library and successfully compile then this standalone setup is obviated.

@nsmith- Does this seem okay as a solution that provides the Python bindings from PyPI when XRootD is already installed? Or do you have a use case that I'm still missing here?

FROM cern/cc7-base:latest

RUN yum update -y && \
    yum install --nogpg -y \
        cmake3 \
        make \
        krb5-devel \
        libuuid-devel \
        libxml2-devel \
        openssl-devel \
        systemd-devel \
        zlib-devel \
        devtoolset-7-gcc-c++ \
        which \
        python3-devel \
        python3-setuptools \
        git \
        cppunit-devel && \
    yum clean all

# Find libXrdCl.so.3 once installed under /usr/local
ENV LD_LIBRARY_PATH="/usr/local/lib64:${LD_LIBRARY_PATH}"
# Install XRootD v5.4.2 with no Python bindings
RUN . /opt/rh/devtoolset-7/enable && \
    git clone --depth 1 https://github.com/xrootd/xrootd \
        --branch v5.4.2 \
        --single-branch && \
    cmake3 \
        -DCMAKE_INSTALL_PREFIX=/usr/local/ \
        -S xrootd \
        -B build && \
    cmake3 build -LH && \
    cmake3 \
        --build build \
        --clean-first \
        --parallel $(($(nproc) - 1)) && \
    cmake3 --build build --target install && \
    command -v xrootd && \
    command -v xrdcp && \
    xrdcp --version

# Install XRootD Python bindings from sdist on PyPI
RUN . /opt/rh/devtoolset-7/enable && \
    python3 -m venv venv && \
    . venv/bin/activate && \
    python -m pip install --upgrade pip setuptools wheel && \
    python -m pip install --verbose 'xrootd==5.4.2' && \
    python -m pip list && \
    python -m pip show xrootd && \
    command -v xrootd && \
    command -v xrdcp && \
    xrdcp --version && \
    python -c 'import XRootD; print(XRootD)' && \
    python -c 'import pyxrootd; print(pyxrootd)' && \
    python -c 'from XRootD import client; print(client.FileSystem("root://someserver:1094"))'
$ docker build -f Dockerfile -t xrootd/xrootd:issue-1594 .
$ docker run --rm -ti xrootd/xrootd:issue-1594 /bin/bash
[root@ddbe76d3b2f7 /]# . venv/bin/activate
(venv) [root@ddbe76d3b2f7 /]# xroot -v
v5.4.2
(venv) [root@ddbe76d3b2f7 /]# xrdcp --version
v5.4.2
(venv) [root@ddbe76d3b2f7 /]# python -m pip show xrootd  # PyPI metadata is missing from sdist at the moment
WARNING: No metadata found in /venv/lib/python3.6/site-packages
WARNING: No metadata found in /venv/lib/python3.6/site-packages
WARNING: No metadata found in /venv/lib/python3.6/site-packages
Name: xrootd
Version: 5.4.2
Summary: 
Home-page: 
Author: 
Author-email: 
License: 
Location: /venv/lib/python3.6/site-packages
Requires: 
Required-by: 
(venv) [root@ddbe76d3b2f7 /]# python -c 'from XRootD import client; print(client.FileSystem("root://someserver:1094"))'
<XRootD.client.filesystem.FileSystem object at 0x7f9fb1787128>
(venv) [root@ddbe76d3b2f7 /]# 

It this meets your use cases then I think we could remove bindings/python/setup_standalone.py unless this would break anyone's workflow.

@matthewfeickert
Copy link
Contributor Author

gentle ping @nsmith- (I know you and I have both been very busy as of late with Snowmass)

@nsmith-
Copy link
Contributor

nsmith- commented Mar 16, 2022

Looks good to me, seems python -m pip install --verbose 'xrootd==5.4.2' is the magic line I was hoping for

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 a pull request may close this issue.

3 participants