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

Automatically detect all cores when the --use-all-cores flag is passed #227

Open
wants to merge 60 commits into
base: master
Choose a base branch
from

Conversation

ghost
Copy link

@ghost ghost commented Jul 8, 2022

As title says - I decided not to pass the custom flags for building bindings, because I know sometimes multithread compilation sometimes can break stuff - if I will get free time I will test it with it too.

karolyi and others added 30 commits March 26, 2018 04:36
Django < 1.11 is no longer supported by Django or Haystack

Test against latest versions of Django 1.11, 2.0, 2.1.
Test against python 2.7 (django 1.11 only), 3.5, 3.6, 3.7 (on xenial).

Update xapian old-stable to 1.2.25, only for py27.
Update xapian old-dev to 1.3.6, only for py27, py34.
Update xapian stable to 1.4.9.

xapian 1.3.6 included just to keep coverage numbers up.
xapian 1.3.7 not included because it didn't build with install-xapian.sh.
This commit could use scrutiny.
check if user supplied version on command line and exit with message if they didn't
asedeno and others added 25 commits September 24, 2021 19:57
1) `xapian_wheel_builder.sh` adds `xapian-delve` to the wheel
2) `tests/xapian_tests/tests/test_backend.py` learns to check the new `xapian-delve` location
3) drop support for delve from xapian<1.3 (support dropped along Python 2)
* xapian_wheel_builder - better metadata

Fixes: notanumber#203
Use `coverage combine` with some more configuration in .coveragerc
to merge our own paths with the paths we copy to when running coverage
and tests in the django-haystack checkout.

Also, print the simple coverage report in the Github Actions workflow
for good measure.

Fixes: notanumber#205
xapian-haystack now lists xapian 1.4.x as the minimum supported
version, so we don't need to carry around code to support 1.2.x
anymore.
Django 3.2.9 added support for python 3.10, but older versions still
list the maximum supported version as 3.9.

https://docs.djangoproject.com/en/3.2/faq/install/

This adds python 3.10 / django 3.2 to the test matrix, and builds a
xapian wheel for python 3.10.

Version numbers changed to strings for consistency. Python 3.10 needs
to be quoted because it would otherwise be interpreted as 3.1.
This reverts commit c624876.

Coverage upstream has fixed the issue this was working around.
xapian issue #364 is solved in xapian 1.3+.
Drop Django 3.1 (EoL)
Add Django 4.0
Add Python 3.8 (Minimum version for Django 4.0)
Add Python 3.10 to the general matrix, not just for Djagno 3.2

Exclude Django 2.2 from Python 3.10 testing.
Exclude Django 4.0 from Python 3.7 testing.

Django 2.2 will EoL in a little over a month, at which point we can
trim the matrix down again by dropping it and Python 3.9 if we want
to.
# Conflicts:
#	MANIFEST.in
#	README.rst
#	install_xapian.sh
#	setup.py
#	xapian_backend.py
@claudep claudep requested a review from asedeno July 8, 2022 06:49
Copy link
Collaborator

@asedeno asedeno left a comment

Choose a reason for hiding this comment

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

Seeing this in isolation and added to xapian_wheel_builder.sh, I think it needs a little more work, though I remain eager to see it polished and merged.

xapian_wheel_builder.sh already has some option processing of its own and this should be folded into it, though it may need some work to support long arguments.

install_xapian.sh should also grow some option processing so it's not brittle in the face of ordering. (install_xapian.sh 1.4.18 --use-all-cores works, but install_xapian.sh --use-all-cores 1.4.18 fails.)

For both scripts, once option processing is handled better, the usage information printed out when no arguments are specified should be updated.

Comment on lines +11 to +14
JFLAG=1
if [ "$2" = "--use-all-cores" ]; then
JFLAG=$(($(getconf _NPROCESSORS_ONLN) + 1))
fi
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems to work on current macOS and FreeBSD, though some older FreeBSD may only have NPROCESSORS_ONLN (no leading underscore). That seems to fail in a way that limits us to 1 processor and then proceeds, so that's probably fine.

@asedeno
Copy link
Collaborator

asedeno commented Jul 9, 2022

GNU getopt is great for this, but BSD getopt doesn't do long options, and we want this to be portable. Bash's getopts builtin also doesn't seem to do long options without a lot of coaxing. Thoughts on a short option for this? I'm thinking -j, though I have reservations since it would not behave like the -j that we're passing to make, even though it is influencing it.

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.

7 participants