Skip to contentSkip to conversation

Fixes and package improvement to Travis CI

PATCH In review Leon Romanovsky 3 participants 8 messages Nov 13, 2017 – Nov 14, 2017 4 messages not in the archive InfiniBand

lore
Share & tools
lore
#0 lore raw

cover Fixes and package improvement to Travis CI

Hi,

This patchset comes from Alaa, who fixed two issues discovered during his work to enable checks of distribution packages in Travis CI. These fixes allowed us to add Ubuntu Artful to the list of checked distributions.

PR can be found at:
https://github.com/linux-rdma/rdma-core/pull/248

Cc Benjamin Drung <[email protected]>
Cc Alaa Hleihel <alaa-VPRAkNaXOzVWk0Htik3J/[email protected]>
Cc Jason Gunthorpe <jgg-uk2M96/[email protected]>
Cc Sean Hefty <[email protected]>
Cc Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/[email protected]>
Cc Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/[email protected]>
Cc Nicolas Morey-Chaisemartin <[email protected]>

Thanks

Alaa Hleihel (3):
Fix compilation warnings when NDEBUG is set
util/mmio.c: Fix compilation errors when __SSE__ is not set on 32 bit
travis: Test Debian packaging under artful container

 .travis.yml                 |  6 ------
 buildlib/cbuild             |  7 +++++++
 buildlib/package-build-test |  2 +-
 buildlib/travis-build       | 16 +---------------
 libibverbs/neigh.c          |  2 +-
 librdmacm/rsocket.c         |  4 ++--
 util/mmio.c                 |  4 ++--
 7 files changed, 14 insertions(+), 27 deletions(-)

--
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20171113060806.27354-1-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

#2 lore raw

PATCH 1/3 Fix compilation warnings when NDEBUG is set

From: Alaa Hleihel <alaa-VPRAkNaXOzVWk0Htik3J/[email protected]>

When NDEBUG is set (-DCMAKE_BUILD_TYPE=Release) and CFLAGS is set to -Werorr, we get some compilation errors as the following:

rdma-core/libibverbs/neigh.c:378:13: error: variable 'rc' set but not used [-Werror=unused-but-set-variable]

     ssize_t rc;
---
 libibverbs/neigh.c  | 2 +-
 librdmacm/rsocket.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)
libibverbs/neigh.c +1 -1
index 3e9ee926..21177db0 100644
@@ -375,7 +375,7 @@ static struct nl_addr *process_get_neigh_mac(
 
 			if (FD_ISSET(timer_fd, &fdset)) {
 				uint64_t read_val;
-				ssize_t rc;
+				ssize_t __attribute__((unused)) rc;
 
 				rc =
 				    read(timer_fd, &read_val, sizeof(read_val));
librdmacm/rsocket.c +2 -2
index 693d9eb5..2ce71627 100644
@@ -408,14 +408,14 @@ struct ds_udp_header {
 static void write_all(int fd, const void *msg, size_t len)
 {
 	// FIXME: if fd is a socket this really needs to handle EINTR and other conditions.
-	ssize_t rc = write(fd, msg, len);
+	ssize_t __attribute__((unused)) rc = write(fd, msg, len);
 	assert(rc == len);
 }
 
 static void read_all(int fd, void *msg, size_t len)
 {
 	// FIXME: if fd is a socket this really needs to handle EINTR and other conditions.
-	ssize_t rc = read(fd, msg, len);
+	ssize_t __attribute__((unused)) rc = read(fd, msg, len);
 	assert(rc == len);
 }
 
Signature · 6 lines
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#3 lore raw

PATCH 2/3 util/mmio.c: Fix compilation errors when __SSE__ is not set on 32 bit

In reply to a message not in the archive

From: Alaa Hleihel <alaa-VPRAkNaXOzVWk0Htik3J/[email protected]>

When building for 32 bit, the compilation may fail with the following errors in case that __SSE__ is not set:

In file included from ../util/mmio.c:37:0:
/usr/lib/gcc/x86_64-linux-gnu/4.8/include/xmmintrin.h:31:3: error: #error "SSE instruction set not enabled"

---
 util/mmio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
util/mmio.c +2 -2
index b362a65e..cea4a727 100644
@@ -33,7 +33,7 @@ static void pthread_mmio_write64_be(void *addr, __be64 val)
 	mmio_wc_spinunlock(&mmio_spinlock);
 }
 
-#if defined(__i386__)
+#if defined(__i386__) && defined(__SSE__)
 #include <xmmintrin.h>
 #include <cpuid.h>
 
@@ -73,7 +73,7 @@ write64_fn_t resolve_mmio_write64_be(void) __asm__("mmio_write64_be");
 
 write64_fn_t resolve_mmio_write64_be(void)
 {
-#if defined(__i386__)
+#if defined(__i386__) && defined(__SSE__)
 	if (have_sse())
 		return &sse_mmio_write64_be;
 #endif
Signature · 6 lines
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20171113060806.27354-3-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

#5 lore raw

Re: [PATCH rdma-core 2/3] util/mmio.c: Fix compilation errors when __SSE__ is not set on 32 bit

Quoted 7 lines · Leon Romanovsky

From: Alaa Hleihel <alaa-VPRAkNaXOzVWk0Htik3J/[email protected]>

When building for 32 bit, the compilation may fail with the following errors in case that __SSE__ is not set:

In file included from ../util/mmio.c:37:0:
/usr/lib/gcc/x86_64-linux-gnu/4.8/include/xmmintrin.h:31:3: error: #error "SSE instruction set not enabled"

Please do not apply this until we fix the root cause.

Debian and RH 32 bit builds should be built with SSE turned on, so they may need some compiler options in their packaging files.

Otherwise all this code is totally useless.

Also, this condition should be a #warning print or some kind of cmake failure as it is deeply undesired..

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#6 lore raw

PATCH 3/3 travis: Test Debian packaging under artful container

In reply to a message not in the archive

From: Alaa Hleihel <alaa-VPRAkNaXOzVWk0Htik3J/[email protected]>

This will be compiled with GCC 7 coming with Ubuntu artful.

Also, remove installations of GCC 7 and debian helpers from addons->apt list in .travis.yml file as they are no longer required.

Finally, since we are removing a -Werror compilation test from buildlib/travis-build, make sure that all builds performed by buildlib/cbuild will run with -Werorr by adding it to CFLAGS environment variable.

---
 .travis.yml                 |  6 ------
 buildlib/cbuild             |  7 +++++++
 buildlib/package-build-test |  2 +-
 buildlib/travis-build       | 16 +---------------
 4 files changed, 9 insertions(+), 22 deletions(-)
.travis.yml +0 -6
index 4d7b0cc0..dadee011 100644
@@ -14,18 +14,13 @@ addons:
       # whitelisted 4.0
       - sourceline: "deb http://apt.llvm.org/trusty/ llvm-toolchain-trusty-4.0 main"
         key_url: "http://apt.llvm.org/llvm-snapshot.gpg.key"
-      - ubuntu-toolchain-r-test
       # Multiverse is not on by default and we need it to get sparse
       - sourceline: "deb http://archive.ubuntu.com/ubuntu/ trusty multiverse"
     packages:
       - build-essential
       - clang-4.0
       - cmake
-      - debhelper
-      - dh-systemd
-      - fakeroot
       - gcc
-      - gcc-7
       - git
       - libnl-3-dev
       - libnl-route-3-dev
@@ -40,7 +35,6 @@ addons:
 
       # 32 bit support packages
       - gcc-multilib
-      - lib32gcc-7-dev
 
 service:
     - docker
buildlib/cbuild +7 -0
index 32bb70df..d47938ef 100755
@@ -478,6 +478,9 @@ os.symlink({tarfn!r},os.path.join("SOURCES",tarfn));
             extra_opts = getattr(env,"rpmbuild_options", [])
             bopts = ["-bb",tspec_file] + extra_opts;
 
+            # Let rpmbuild compile with -Werror
+            print >> F,'os.environ["CFLAGS"] = "-Werror"'
+
             print >> F,'os.execlp("rpmbuild","rpmbuild",%s)'%(
                 ",".join(repr(I) for I in bopts));
 
@@ -533,6 +536,10 @@ import subprocess,os;
 def to_user():
    os.setgid({gid:d});
    os.setuid({uid:d});
+
+# Compile with -Werror
+os.environ["CFLAGS"] = "-Werror"
+
 subprocess.check_call(["debian/rules","debian/rules","build"],
             preexec_fn=to_user);
 subprocess.check_call(["debian/rules","debian/rules","binary"]);
buildlib/package-build-test +1 -1
index 15f24caa..b1a7807c 100755
@@ -11,7 +11,7 @@ if [ -e "/.dockerenv" ] || (grep -q docker /proc/self/cgroup &>/dev/null); then
        exit 0
 fi
 
-for OS in centos7 tumbleweed
+for OS in centos7 tumbleweed artful
 do
 	echo
 	echo "Checking package build for ${OS} ...."
buildlib/travis-build +1 -15
index bf86baa7..3ee4182b 100755
@@ -17,7 +17,7 @@ ninja
 cd ../build32
 # travis's trusty is not configured in a way that enables all 32 bit
 # packages. We could fix this with some sudo stuff.. For now turn off libnl
-CC=gcc-7 CFLAGS="-Werror -m32" cmake -GNinja .. -DENABLE_RESOLVE_NEIGH=0
+CFLAGS="-Werror -m32" cmake -GNinja .. -DENABLE_RESOLVE_NEIGH=0
 ninja
 
 # aarch64 build to check compilation on ARM 64bit platform
@@ -45,17 +45,3 @@ rm CMakeCache.txt
 CC=clang-4.0 CFLAGS=-Werror cmake -GNinja ..
 ninja
 cp ../util/udma_barrier.h.old ../util/udma_barrier.h
-
-# Finally run through gcc-7 64 bit through the debian packaging This gives a
-# good clue if patches are changing packaging related things, the RPM stuff
-# will have to be audited by hand.
-
-# When running cmake through debian/rules it is hard to set -Werror,
-# instead force it on by changing the CMakeLists.txt
-cd ..
-echo 'set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Werror")' >> buildlib/RDMA_EnableCStd.cmake
-sed -i -e 's/-DCMAKE_BUILD_TYPE=Release//g' debian/rules
-sed -i -e 's/ninja \(.*\)-v/ninja \1/g' debian/rules
-
-CC=gcc-7 debian/rules build
-fakeroot debian/rules binary
Signature · 6 lines
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20171113060806.27354-4-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

#8 lore raw

Re: [PATCH rdma-core 3/3] travis: Test Debian packaging under artful container

buildlib/cbuild import subprocess,os; · line 541

Quoted 2 lines · Leon Romanovsky
buildlib/cbuild +2 -0
+# Compile with -Werror
+os.environ["CFLAGS"] = "-Werror"

Please don't force -Werror for all cbuild runs.. We expect to get unfixable warnings from some of the older containers like RH6.

Suggest a command line option to use when called by travis? Or why not just set CFLAGS in travis in the first place?

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#9 lore raw
In reply to a message not in the archive

Le 13/11/2017 à 07:08, Leon Romanovsky a écrit :

Quoted 34 lines

Hi,

This patchset comes from Alaa, who fixed two issues discovered during his work to enable checks of distribution packages in Travis CI. These fixes allowed us to add Ubuntu Artful to the list of checked distributions.

PR can be found at:
https://github.com/linux-rdma/rdma-core/pull/248

Cc Benjamin Drung <[email protected]>
Cc Alaa Hleihel <alaa-VPRAkNaXOzVWk0Htik3J/[email protected]>
Cc Jason Gunthorpe <jgg-uk2M96/[email protected]>
Cc Sean Hefty <[email protected]>
Cc Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/[email protected]>
Cc Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/[email protected]>
Cc Nicolas Morey-Chaisemartin <[email protected]>

Thanks

Alaa Hleihel (3):
Fix compilation warnings when NDEBUG is set
util/mmio.c: Fix compilation errors when __SSE__ is not set on 32 bit
travis: Test Debian packaging under artful container

 .travis.yml                 |  6 ------
 buildlib/cbuild             |  7 +++++++
 buildlib/package-build-test |  2 +-
 buildlib/travis-build       | 16 +---------------
 libibverbs/neigh.c          |  2 +-
 librdmacm/rsocket.c         |  4 ++--
 util/mmio.c                 |  4 ++--
 7 files changed, 14 insertions(+), 27 deletions(-)
Quoted 3 lines

--
2.15.0

For the series:

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 65be5c4f-0127-78c7-db0c-f54c22884902-l3A5Bk7waGM@public.gmane.org

#11 lore raw
Quoted 37 lines · Nicolas Morey-Chaisemartin

Le 13/11/2017 à 07:08, Leon Romanovsky a écrit :

Quoted 34 lines

Hi,

This patchset comes from Alaa, who fixed two issues discovered during his work to enable checks of distribution packages in Travis CI. These fixes allowed us to add Ubuntu Artful to the list of checked distributions.

PR can be found at:
https://github.com/linux-rdma/rdma-core/pull/248

Cc Benjamin Drung <[email protected]>
Cc Alaa Hleihel <alaa-VPRAkNaXOzVWk0Htik3J/[email protected]>
Cc Jason Gunthorpe <jgg-uk2M96/[email protected]>
Cc Sean Hefty <[email protected]>
Cc Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/[email protected]>
Cc Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/[email protected]>
Cc Nicolas Morey-Chaisemartin <[email protected]>

Thanks

Alaa Hleihel (3):
Fix compilation warnings when NDEBUG is set
util/mmio.c: Fix compilation errors when __SSE__ is not set on 32 bit
travis: Test Debian packaging under artful container

 .travis.yml                 |  6 ------
 buildlib/cbuild             |  7 +++++++
 buildlib/package-build-test |  2 +-
 buildlib/travis-build       | 16 +---------------
 libibverbs/neigh.c          |  2 +-
 librdmacm/rsocket.c         |  4 ++--
 util/mmio.c                 |  4 ++--
 7 files changed, 14 insertions(+), 27 deletions(-)
Quoted 6 lines
Quoted 3 lines

--
2.15.0

For the series:

I took first patch, everything else is dropped.

Thanks