Skip to content

Fix runloop integration for libdispatch from swift #87

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.

Sign up for GitHub

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 2 commits into from
Dec 6, 2019
Merged

Fix runloop integration for libdispatch from swift #87

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ecd2d85
Fix runloop integration for libdispatch from swift
ngrewe Nov 26, 2019
b238f81
Merge remote-tracking branch 'origin/master' into swift-corelibs-libd…
ngrewe Dec 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Fix runloop integration for libdispatch from swift
  • Loading branch information
@ngrewe
ngrewe committed Nov 28, 2019
commit ecd2d85cb97a585ced972e99bf90990d9c45bb83
19 changes: 14 additions & 5 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
language: cpp
dist: trusty
dist: xenial
Copy link
Member

@fredkiefer fredkiefer Nov 26, 2019

Choose a reason for hiding this comment

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

Should we update even further?

Copy link
Member Author

@ngrewe ngrewe Nov 26, 2019

Choose a reason for hiding this comment

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

Of course, that makes sense. I was living in the past… and forgot that bionic is the newest Ubuntu LTS version.

Copy link
Member Author

@ngrewe ngrewe Nov 26, 2019

Choose a reason for hiding this comment

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

Interestingly, that gives us some unrelated test failures for NSURL-related things 🤔. I guess I'll revert to xenial for the time being and investigate those independently....

compiler:
- clang
- gcc
Expand All @@ -24,22 +24,31 @@ matrix:
sudo: required
before_install:
- sudo apt-get -qq update
- sudo apt-get install -y cmake pkg-config libgnutls28-dev libgmp-dev libffi-dev libicu-dev libxml2-dev libxslt1-dev libssl-dev libavahi-client-dev zlib1g-dev libblocksruntime-dev
- sudo apt-get install -y cmake pkg-config libgnutls28-dev libgmp-dev libffi-dev libicu-dev libxml2-dev libxslt1-dev libssl-dev libavahi-client-dev zlib1g-dev
- >
if [ $LIBRARY_COMBO = 'gnu-gnu-gnu' ];
then
if [ $CC = 'gcc' ];
then
sudo apt-get install -y gobjc;
fi;
sudo apt-get install -y libobjc-4.8-dev;
sudo apt-get install -y libobjc-4.8-dev libblocksruntime-dev;
else
sudo apt-get install -y libkqueue-dev libpthread-workqueue-dev;
fi;
# libdispatch requires a fairly recent version of cmake
- >
if [ $LIBRARY_COMBO = 'ng-gnu-gnu' ];
then
curl -LO https://cmake.org/files/v3.15/cmake-3.15.5-Linux-x86_64.tar.gz;
tar xf cmake-3.15.5-Linux-x86_64.tar.gz;
mv cmake-3.15.5-Linux-x86_64 $HOME/cmake;
export PATH=$HOME/cmake/:$HOME/cmake/bin:$PATH
fi;
install: ./travis-deps.sh
before_script: >
export LIBRARY_PATH=$HOME/staging/lib:$LIBRARY_PATH;
export LD_LIBRARY_PATH=$HOME/staging/lib:$LD_LIBRARY_PATH;
export LIBRARY_PATH=$HOME/staging/lib:$HOME/staging/lib64:$LIBRARY_PATH;
export LD_LIBRARY_PATH=$HOME/staging/lib:$HOME/staging/lib64:$LD_LIBRARY_PATH;
if [ $LIBRARY_COMBO = 'ng-gnu-gnu' ];
then
export CPATH=$HOME/staging/include;
Expand Down
9 changes: 9 additions & 0 deletions ChangeLog
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
2019-11-26 Niels Grewe <[email protected]>

* Source/NSRunLoop.m:
* Tests/base/NSRunLoop/dispatch.m:
Fix runloop integration for libdispatch from swift-corelibs
* .travis.yml:
* travis-deps.yml:
Fix CI related to libdispatch.

2019-11-15 Frederik Seiffert <[email protected]>

* configure.ac: check for unwind.h
Expand Down
6 changes: 3 additions & 3 deletions Source/NSRunLoop.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
#include <math.h>
#include <time.h>

#if HAVE_LIBDISPATCH_RUNLOOP
#if GS_USE_LIBDISPATCH_RUNLOOP
# define RL_INTEGRATE_DISPATCH 1
# ifdef HAVE_DISPATCH_H
# include <dispatch.h>
Expand Down Expand Up @@ -403,7 +403,7 @@ + (void*) mainQueueFileDescriptor
#if HAVE_DISPATCH_GET_MAIN_QUEUE_HANDLE_NP
return (void*)(uintptr_t)dispatch_get_main_queue_handle_np();
#elif HAVE__DISPATCH_GET_MAIN_QUEUE_HANDLE_4CF
return (void*)_dispatch_get_main_queue_handle_4CF();
return (void*)(uintptr_t)_dispatch_get_main_queue_handle_4CF();
#else
#error libdispatch missing main queue handle function
#endif
Expand All @@ -417,7 +417,7 @@ - (void) receivedEvent: (void*)data
#if HAVE_DISPATCH_MAIN_QUEUE_DRAIN_NP
dispatch_main_queue_drain_np();
#elif HAVE__DISPATCH_MAIN_QUEUE_CALLBACK_4CF
_dispatch_main_queue_callback_4CF(NULL)
_dispatch_main_queue_callback_4CF(NULL);
#else
#error libdispatch missing main queue callback function
#endif
Expand Down
8 changes: 3 additions & 5 deletions Tests/base/NSRunLoop/dispatch.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,12 @@

const NSTimeInterval kDelay = 0.01;

#if HAVE_LIBDISPATCH_RUNLOOP && __has_feature(blocks)
#if GS_USE_LIBDISPATCH_RUNLOOP && __has_feature(blocks)
Copy link
Member

@fredkiefer fredkiefer Nov 26, 2019

Choose a reason for hiding this comment

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

Wouldn't it be easier to keep this code here identical to the one in NSRunLoop.m?

Copy link
Member Author

@ngrewe ngrewe Nov 26, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

In the implementation, we only need to call C functions from libdispatch, but in the test, we also need to schedule blocks onto the main queue to verify that running the runloop will drain the queue as well. Hence the subtle difference: The test code depends on a blocks capable compiler, the implementation code doesn't.

Copy link
Member

@fredkiefer fredkiefer Nov 26, 2019

Choose a reason for hiding this comment

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

Sorry, I was rather aiming at the lines below, not that one by itself.

Copy link
Member Author

@ngrewe ngrewe Nov 26, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Ah, got it: The HAVE_DISPATCH_H defines etc. come from Source/config.h, so it doesn't seem like they were supposed to be exposed to the test framework (otherwise it should be in a Headers/ subdirectory -- probably one of the reasons why this code path broke down with time). And since this is effectively a clang only code path, it made sense to do it using __has_include() here.

This is very defensive anyways: I haven't seen a build of libdispatch that didn't place its headers in subdirectory for a long time.

# define DISPATCH_RL_INTEGRATION 1
# ifdef HAVE_DISPATCH_H
# if __has_include(<dispatch.h>)
# include <dispatch.h>
# else
# ifdef HAVE_DISPATCH_DISPATCH_H
# include <dispatch/dispatch.h>
# endif
# include <dispatch/dispatch.h>
# endif
#endif

Expand Down
22 changes: 12 additions & 10 deletions travis-deps.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
set -ex

DEP_SRC=$HOME/dependency_source/
DEP_ROOT=$HOME/staging

install_gnustep_make() {
cd $DEP_SRC
Expand All @@ -12,7 +13,7 @@ install_gnustep_make() {
then
echo "RUNTIME_VERSION=$RUNTIME_VERSION" > GNUstep.conf
fi
./configure --prefix=$HOME/staging --with-library-combo=$LIBRARY_COMBO --with-user-config-file=$PWD/GNUstep.conf
./configure --prefix=$DEP_ROOT --with-library-combo=$LIBRARY_COMBO --with-user-config-file=$PWD/GNUstep.conf
make install
echo Objective-C build flags: `$HOME/staging/bin/gnustep-config --objc-flags`
}
Expand All @@ -30,26 +31,27 @@ install_ng_runtime() {
export CC="clang"
export CXX="clang++"
export CXXFLAGS="-std=c++11"
cmake -DTESTS=off -DCMAKE_BUILD_TYPE=RelWithDebInfo -DGNUSTEP_INSTALL_TYPE=NONE -DCMAKE_INSTALL_PREFIX:PATH=$HOME/staging ../
cmake -DTESTS=off -DCMAKE_BUILD_TYPE=RelWithDebInfo -DGNUSTEP_INSTALL_TYPE=NONE -DCMAKE_INSTALL_PREFIX:PATH=$DEP_ROOT ../
make install
}

install_libdispatch() {
cd $DEP_SRC
git clone https://github.com/ngrewe/libdispatch.git
mkdir libdispatch/build
cd libdispatch/build
# will reference upstream after https://github.com/apple/swift-corelibs-libdispatch/pull/534 is merged
git clone -b system-blocksruntime https://github.com/ngrewe/swift-corelibs-libdispatch.git
mkdir swift-corelibs-libdispatch/build
cd swift-corelibs-libdispatch/build
export CC="clang"
export CXX="clang++"
export LIBRARY_PATH=$HOME/staging/lib;
export LD_LIBRARY_PATH=$HOME/staging/lib:$LD_LIBRARY_PATH;
export CPATH=$HOME/staging/include;
cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_INSTALL_PREFIX:PATH=$HOME/staging ../
export LIBRARY_PATH=$DEP_ROOT/lib;
export LD_LIBRARY_PATH=$DEP_ROOT/lib:$LD_LIBRARY_PATH;
export CPATH=$DEP_ROOT/include;
cmake -DBUILD_TESTING=off -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_INSTALL_PREFIX:PATH=$HOME/staging -DINSTALL_PRIVATE_HEADERS=1 -DBlocksRuntime_INCLUDE_DIR=$DEP_ROOT/include -DBlocksRuntime_LIBRARIES=$DEP_ROOT/lib/libobjc.so ../
make install
}

mkdir -p $DEP_SRC
if [ $LIBRARY_COMBO = 'ng-gnu-gnu' ]
if [ "$LIBRARY_COMBO" = 'ng-gnu-gnu' ]
then
install_ng_runtime
install_libdispatch
Expand Down