Skip to content

speedup osx build with ccache and other stuff #1

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

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

sergregory
Copy link
Owner

No description provided.

)
for m in ${CV_MODULES[@]}; do
if make help | grep $m; then
time make -j $m
Copy link
Owner Author

Choose a reason for hiding this comment

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

I think it should be something like time make -j$(sysctl -n hw.ncpu) $m. With just -j option make will launch as many threads as possible, which, in many situations, slower than launching 1 thread per core.

time make -j $m
fi
done
time make -j
Copy link
Owner Author

Choose a reason for hiding this comment

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

And the same comment as above here.

ENABLE_HEADLESS=0
ENABLE_JAVA=0
CWD=`pwd`
REPO=${REPO:-/Users/grigoryserebryakov/Documents/project/python-opencv/repos}
Copy link
Owner Author

Choose a reason for hiding this comment

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

Looks strange to me - ~ means home directory, so probably it should be either
~/Documents/project/python-opencv/repos or $HOME/Documents/project/python-opencv/repos

ENABLE_CONTRIB=1
ENABLE_HEADLESS=0
ENABLE_JAVA=0
CWD=`pwd`
Copy link
Owner Author

@sergregory sergregory Dec 28, 2020

Choose a reason for hiding this comment

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

Please use the shellcheck tool and fix the things it warns you about

# Install python 3.7-3.9
# brew install [email protected]
brew install [email protected]
brew install [email protected]
Copy link
Owner Author

Choose a reason for hiding this comment

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

Looks strange to me - why should we install several pythons just to unlink one of them and use another one?

ENABLE_CONTRIB=1
ENABLE_HEADLESS=0
ENABLE_JAVA=0
REPO=${REPO:-/Users/grigoryserebryakov/Documents/project/python-opencv/repos}
Copy link
Owner Author

Choose a reason for hiding this comment

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

Comment about the home directory

export ENABLE_CONTRIB
export ENABLE_HEADLESS
export ENABLE_JAVA
pip3 wheel .
Copy link
Owner Author

Choose a reason for hiding this comment

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

this is a bit confusing - surely you can do pip3 wheel, but I'd expect python3 setup.py bdist_wheel here - at least because you already use setup.py the same way for sdist several lines above

@@ -0,0 +1,23 @@
import sys
Copy link
Owner Author

Choose a reason for hiding this comment

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

Probably, it should be executable, so needs sha-bang and the '+x' flag

@sergregory
Copy link
Owner Author

@GArik please take a look

if (sys.argv[1] == "bin"):
print(sys.executable)
elif (sys.argv[1] == "lib"):
python_lib_path = cmaker.CMaker.get_python_library(python_version).replace("\\", "/")
Copy link
Owner Author

Choose a reason for hiding this comment

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

@GArik GArik force-pushed the osx branch 6 times, most recently from a85b34c to 457c377 Compare January 18, 2021 12:30
GArik added 4 commits January 18, 2021 21:31
To fix warnings like this:

Warning: You are using macOS 10.13.
5We (and Apple) do not provide support for this old version.
You will encounter build failures with some formulae.

Also use Qt-5.15.2 which is installed in MacOS 10.15.5
Use cmake && make to prebuild opencv libraries and use pip to pack them
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.

2 participants