Comments (8)
Makefile could check whether that virtual environment was active outside of make and skip the removal then.
Sounds reasonable 👍 @bertsky any consequences we're not seeing?
from ocrd_all.
make clean
removes venv directory in source path unconditionally
Ideally it would only remove files created by the build process.If the user has created the
venv
directory manually and added own content, removing that directory might be bad.
In the current rule definition, clean
only cleanes the path ocrd_all uses by default to create a new venv of its own. It does not use VIRTUAL_ENV
from the environment (which might be user-defined).
If both coincide, then it is removed. So you want to back out of that decision?
Makefile could check whether that virtual environment was active outside of make and skip the removal then.
What do you propose? Replace the VIRTUAL_ENV ?=
with some form of ifdef
?
from ocrd_all.
What do you propose?
--- a/Makefile
+++ b/Makefile
@@ -69,7 +69,7 @@ check:
clean: # add more prerequisites for clean below
$(RM) -r $(SUB_VENV)/*
- $(RM) -r $(CURDIR)/venv # deliberately not using VIRTUAL_ENV here
+ if ! command -v activate; then $(RM) -r $(VIRTUAL_ENV; fi
$(RM) -r $(HOME)/.parallel/semaphores/id-ocrd_all_git/
define HELP
Maybe a comment can also be added: "Remove virtual environment only if it was not active outside of the make context.".
from ocrd_all.
if ! command -v activate; then $(RM) -r $(VIRTUAL_ENV; fi
I don't think activate
will be detected as a command because while in $PATH if $VIRTUAL_ENV is activated it is not executable.
Wrong, it does work, should have checked.
from ocrd_all.
Improved version:
diff --git a/Makefile b/Makefile
index d10e36a..cd7751c 100644
--- a/Makefile
+++ b/Makefile
@@ -69,7 +69,8 @@ check:
clean: # add more prerequisites for clean below
$(RM) -r $(SUB_VENV)/*
- $(RM) -r $(CURDIR)/venv # deliberately not using VIRTUAL_ENV here
+ # Remove virtual environment only if it was not active outside of the make context.
+ command -v activate >/dev/null || $(RM) -r $(VIRTUAL_ENV)
$(RM) -r $(HOME)/.parallel/semaphores/id-ocrd_all_git/
define HELP
from ocrd_all.
Wrong, it does work, should have checked.
I was also surprised about that.
from ocrd_all.
- $(RM) -r $(CURDIR)/venv # deliberately not using VIRTUAL_ENV here + # Remove virtual environment only if it was not active outside of the make context. + command -v activate >/dev/null || $(RM) -r $(VIRTUAL_ENV)
That's not correct. This would even snatch away an externally controlled venv passed in via VIRTUAL_ENV
(either from shell variable or by make argument or in local.mk). You should at least use $(CURDIR)/venv
. But a better criterion than activate
(which could be anything) is the ifdef approach suggested above.
from ocrd_all.
Well, it does not snatch if the external VIRTUAL_ENV
was active when make clean
was called.
But I agree that using macros which come from outside with $(RM) -r
can be dangerous, so feel free to use $(CURDIR)/venv
to avoid trouble coming from someone running make clean VIRTUAL_ENV=/
.
from ocrd_all.
Related Issues (20)
- /models not working HOT 6
- Provide date-based alias for maximum-git
- frak models in ocrd resmgr HOT 27
- empty OCR HOT 13
- model download in Docker only allowed for root HOT 4
- no word coordinates? HOT 6
- Docker: interference with older versions of core HOT 3
- Docker: build CD images sequentially HOT 1
- Use annotated tags for new releases
- `make check` fails with latest code because of missing ocrd-tesserocr-binarize HOT 5
- 2nd build stops waiting for user input HOT 1
- ocrd-import does not work HOT 4
- Docker: Logfile permissions problems HOT 3
- `make check` fails since January
- `make all` fails for Python 3.7 HOT 4
- Docker: build multi-architecture images HOT 3
- qurator namespace pkg problems are back HOT 4
- Broken build (ocrd_detectron2) HOT 2
- Broken builds on Ubuntu 20.04 HOT 3
- Check core submodule version is consistent with base image core version
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from ocrd_all.