Skip to content

ensure venv for Python tools#162

Merged
stweil merged 6 commits intoUB-Mannheim:masterfrom
bertsky:with-venv
Jun 6, 2023
Merged

ensure venv for Python tools#162
stweil merged 6 commits intoUB-Mannheim:masterfrom
bertsky:with-venv

Conversation

@bertsky
Copy link
Copy Markdown
Contributor

@bertsky bertsky commented May 5, 2023

No description provided.

@bertsky bertsky mentioned this pull request May 5, 2023
@bertsky
Copy link
Copy Markdown
Contributor Author

bertsky commented Jun 6, 2023

It's been one month – can we please get ahead with this? (We still need to update ocrd_fileformat...)

@stweil stweil merged commit 3e32ef6 into UB-Mannheim:master Jun 6, 2023
Comment on lines +62 to +67
# create+activate a Python venv if not already active
if [ -z "$(VIRTUAL_ENV)" ]; then \
$(PYTHON) -m venv $(SHAREDIR)/venv && \
. $(SHAREDIR)/venv/bin/activate && \
pip install -U pip; \
fi && $(MAKE) -C vendor all
Copy link
Copy Markdown
Member

@stweil stweil Feb 13, 2024

Choose a reason for hiding this comment

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

@bertsky, @kba, these lines break make all in a default build (VIRTUAL_ENV not set, SHAREDIR=/usr/local/share).

Making a virtual Python environment will fail because of missing write permission.

Using sudo will make things even worse because of git conflicts with different users, and it will also create files owned by root in the user's file tree.

Is it okay if SHAREDIR is writable and already contains a venv to overwrite it by a new create command?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

IMO $(or $(VIRTUAL_ENV), $(SHAREDIR)/venv) should be an explicit prerequisite of vendor (to rule out overwriting and save time).

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.

3 participants