Skip to content

Dingpf/python_venv #51

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

Merged
merged 15 commits into from
Nov 3, 2020
Merged

Dingpf/python_venv #51

merged 15 commits into from
Nov 3, 2020

Conversation

dingp
Copy link
Contributor

@dingp dingp commented Nov 2, 2020

Adding a script to automatically check/setup/initialize/activate a python virtual environment for running "moo".

Details of implementation can be found in issue #50

@dingp dingp linked an issue Nov 2, 2020 that may be closed by this pull request
@dingp
Copy link
Contributor Author

dingp commented Nov 2, 2020

Instructions to test this pull request:

curl -O https://raw.githubusercontent.com/DUNE-DAQ/daq-buildtools/dingpf/python-venv/bin/quick-start.sh
chmod +x quick-start.sh
./quick-start.sh
source ./setup_build_environment
source  setup_python_venv

@dingp
Copy link
Contributor Author

dingp commented Nov 2, 2020

I can have "source setup_python_venv" inside "source ./setup_build_environment" if we want developers to have the virtualenv by default.

But right now, sourcing the "setup_python_venv" is a separate step as shown in my previous comment.

Comment on lines +16 to +17
# Comment out moo, as by default PyPI's moo will be picked up.
# moo==0.2.0
Copy link
Member

Choose a reason for hiding this comment

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

You might be able to replace these lines with one like:

git+git://github.com/brettviren/[email protected]#egg=moo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. I tried it in this commit 4ec2be3.

But it looks like pip was still checking the moo in PyPI. I got the error messages as the following:

  ERROR: Could not find a version that satisfies the requirement moo==git+https://github.com/brettviren/[email protected]#egg=moo (from -r pyvenv_requirements.txt (line 17)) (from versions: 0.1.0, 0.1.1, 0.1.2, 0.1.5, 0.1.7, 0.1.8, 0.2.0, 0.5.3, 0.5.4, 0.5.5)
  ERROR: No matching distribution found for moo==git+https://github.com/brettviren/[email protected]#egg=moo (from -r pyvenv_requirements.txt (line 17))      

Pengfei Ding and others added 5 commits November 2, 2020 23:01
… special handling of moo in the setup_python_env script

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
remove merge conflicts
@bieryAtFnal
Copy link

Pengfei,
I used the instructions that you provided, and they worked successfully to create a python virtual environment that contained moo 0.3.0. That was all great.
Question/comment: on subsequent logins, we'll need to re-activate the moo_venv 'by hand', along with sourcing setup_build_environment, right? Do you have thoughts about when that might become part of the setup_build_environment script?
Thanks,
Kurt

@jcfreeman2
Copy link
Collaborator

jcfreeman2 commented Nov 3, 2020 via email

@dingp
Copy link
Contributor Author

dingp commented Nov 3, 2020

Hi Kurt,

The setup_python_venv is designed to be like a real setup script which you can run after each login. So as of right now on subsequent logins, you can source the script right after you source setup_build_environment to activate the moo_venv.

The advantage of this comparing to activate the env by hand is, the setup script will check/overwrite modules listed in "pyvenv_requirements.txt". You can turn this feature off in the script. There's a flag controlling this at the top.

Best,
Pengfei

Pengfei,
I used the instructions that you provided, and they worked successfully to create a python virtual environment that contained moo 0.3.0. That was all great.
Question/comment: on subsequent logins, we'll need to re-activate the moo_venv 'by hand', along with sourcing setup_build_environment, right? Do you have thoughts about when that might become part of the setup_build_environment script?
Thanks,
Kurt

@dingp
Copy link
Contributor Author

dingp commented Nov 3, 2020

Thanks for pointing this out, John. I think we need to move things to python3. I found cpplint in PyPI does have a variant for python3.8, the current default python version we have in cvmfs.

If we want to have setup_for_build sources setup_python_venv, we can have cpplint installed in the virtual evn, so that build_daq_software.sh --lint can have access to it, and remove the current copy of cpplint.py using python2.7.

Alternatively, we can upgrade current copy of cpplint.py to the python 3.8 compatible version.

Best,
Pengfei

An argument against (immediately) setting up the moo_venv inside of setup_build_environment: if you do this, and you run build_daq_software.sh with the "--lint" option, you'll get this: ERROR: you're not using Python 2.7. Google's cpplint.py, and by extension DUNE's dunecpplint.py, needs Python 2.7 to work. Exiting... ...which is hopefully self-explanatory. True, users can type "deactivate" to go back to their system's python (2.7 on lxplus, mu2edaq, etc.), and true, users can simply read the full style guide, understand it, and apply it, but I think you'll agree that would be pretty optimistic.

@dingp
Copy link
Contributor Author

dingp commented Nov 3, 2020

If we want to have setup_for_build sources setup_python_venv, we can have cpplint installed in the virtual evn, so that build_daq_software.sh --lint can have access to it, and remove the current copy of cpplint.py using python2.7.

This would not work. I forgot the fact that we used a modified version of google's cpplint.py. So we may need to put effort into upgrading our variant to be python3.8 compatible.

Looking at this pull request to official google's style guide repo, it seems the upgrade does not involve much code change.

google/styleguide#528
google/styleguide@9670c3d

John Freeman added 2 commits November 3, 2020 16:06
…this is meant to be sourced, so that you don't get logged out of the shell if there's a problem but simply returned to the prompt
…on-venv; will need to revert this after merging the pull request"

This reverts commit 8987be6.
@jcfreeman2
Copy link
Collaborator

Approving this pull request. The linter only gets broken if users source setup_python_venv, meaning the current daq-buildtools develop branch instructions still hold (https://github.com/DUNE-DAQ/appfwk/wiki/Compiling-and-running).

Otherwise, this appears to do what it's meant to do; I was able, e.g., to rerun generate.sh in appfwk with the moo set up by setup_python_venv. One tweak I made was to have setup_python_venv call return rather than exit when there's a problem, because since it's sourced you'd get logged out by an exit.

@jcfreeman2 jcfreeman2 merged commit c1a721f into develop Nov 3, 2020
@jcfreeman2 jcfreeman2 deleted the dingpf/python-venv branch November 3, 2020 22:19
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.

Setup script and requirements file for python virtualenv
4 participants