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.

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

Jump to bottom

DAOS-16621 build: Fix Go versions in rpm/deb packaging #15174

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
+43 −12
Open

DAOS-16621 build: Fix Go versions in rpm/deb packaging #15174

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a237458
DAOS-16621 build: Fix Go versions in rpm/deb packaging
kjacque Sep 23, 2024
f17c1d5
Address review comments
kjacque Sep 24, 2024
2942650
Update Ubuntu install script
kjacque Sep 24, 2024
408e2d4
Add gpg-agent to ubuntu install
kjacque Sep 25, 2024
2b4106c
Fix pylint error
kjacque Sep 25, 2024
95f6cd3
Harden minimum Go version detection
kjacque Sep 26, 2024
b1ee4cf
Address review comments
kjacque Sep 26, 2024
11ac499
Move add-apt-repository call to Dockerfile
kjacque Sep 27, 2024
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
6 changes: 6 additions & 0 deletions debian/changelog
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
daos (2.7.100-6) unstable; urgency=medium
[ Kris Jacque ]
* Bump minimum golang-go version to 1.21

-- Kris Jacque <kris.jacque@intel.com> Mon, 23 Sep 2024 11:06:00 -0700

daos (2.7.100-5) unstable; urgency=medium
[ Michael MacDonald ]
* Add libdaos_self_test.so to client package
Expand Down
4 changes: 2 additions & 2 deletions debian/control
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Build-Depends: debhelper (>= 10),
dpdk-dev (>= 21.11.2),
libisal-crypto-dev,
libcunit1-dev,
golang-go (>= 1.18),
golang-go (>= 2:1.21),
libboost-dev,
libspdk-dev (>= 22.01.2),
libipmctl-dev,
Expand Down Expand Up @@ -117,7 +117,7 @@ Depends: python (>=3.8), python3, python-yaml, python3-yaml,
${shlibs:Depends}, ${misc:Depends},
daos-client (= ${binary:Version}),
daos-admin (= ${binary:Version}),
golang-go (>=1.18),
golang-go (>= 2:1.21),
libcapstone-dev
Description: The Distributed Asynchronous Object Storage (DAOS) is an open-source
software-defined object store designed from the ground up for
Expand Down
27 changes: 22 additions & 5 deletions site_scons/site_tools/go_builder.py
View file
Open in desktop
kjacque marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@
import re
import subprocess # nosec B404

from SCons.Script import Configure, Exit, File, GetOption, Glob, Scanner
from SCons.Script import Configure, Dir, Exit, File, GetOption, Glob, Scanner

GO_COMPILER = 'go'
MIN_GO_VERSION = '1.18.0'
kjacque marked this conversation as resolved.
Show resolved Hide resolved
include_re = re.compile(r'\#include [<"](\S+[>"])', re.M)


Expand Down Expand Up @@ -49,6 +48,17 @@ def _scan_go_file(node, env, _path):
return includes


def get_min_go_version():
"""Get go minimum version from go.mod"""
go_mod_path = os.path.join(Dir('#').abspath, "src", "control", "go.mod")
with open(go_mod_path, 'r') as f:
for line in f:
if line.startswith('go '): # e.g. "go 1.21"
parts = line.split()
return get_go_version("go" + parts[1])
return None


def get_go_version(output):
"""Capture only the version after 'go'"""
ver_re = re.compile(r'go([0-9\.]+)')
Expand Down Expand Up @@ -81,6 +91,13 @@ def _check_go_version(context):
context.Result(0)
return 0

context.Display('Getting minimum go version... ')
min_go_version = get_min_go_version()
ashleypittman marked this conversation as resolved.
Show resolved Hide resolved
if min_go_version is None:
context.Result('no minimum go version found in go.mod')
return 0
context.Display(min_go_version + '\n')

context.Display(f'Checking {env.d_go_bin} version... ')
cmd_rc = subprocess.run([env.d_go_bin, 'version'], check=True, stdout=subprocess.PIPE)
out = cmd_rc.stdout.decode('utf-8').strip()
Expand All @@ -93,11 +110,11 @@ def _check_go_version(context):
if go_version is None:
context.Result(f'failed to get version from "{out}"')
return 0
if len([x for x, y in
zip(go_version.split('.'), MIN_GO_VERSION.split('.'))
if len([x for x, y in zip(go_version.split('.'), min_go_version.split('.'))
if int(x) < int(y)]) > 0:
context.Result(f'{out} is too old (min supported: {MIN_GO_VERSION}) ')
context.Result(f'{out} is too old (min supported: {min_go_version}) ')
return 0

context.Result(go_version)
return 1

Expand Down
6 changes: 4 additions & 2 deletions src/control/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
module github.com/daos-stack/daos/src/control

// NB: When updating minimum Go build version, don't forget to update:
// - rpm packaging version checks: utils/rpms/daos.spec
// - debian packaging version checks: debian/control
// Scons uses this file to extract the minimum version.
go 1.21

toolchain go1.22.3

require (
github.com/Jille/raft-grpc-transport v1.2.0
github.com/desertbit/grumble v1.1.3
Expand Down
9 changes: 6 additions & 3 deletions utils/rpms/daos.spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ BuildRequires: libyaml-devel
BuildRequires: libcmocka-devel
BuildRequires: valgrind-devel
BuildRequires: systemd
BuildRequires: go >= 1.17
BuildRequires: go >= 1.21
BuildRequires: pciutils-devel
%if (0%{?rhel} >= 8)
BuildRequires: numactl-devel
Expand Down Expand Up @@ -218,7 +218,7 @@ Requires: dbench
Requires: lbzip2
Requires: attr
Requires: ior
Requires: go >= 1.18
Requires: go >= 1.21
%if (0%{?suse_version} >= 1315)
Requires: lua-lmod
Requires: libcapstone-devel
Expand Down Expand Up @@ -592,6 +592,9 @@ getent passwd daos_agent >/dev/null || useradd -s /sbin/nologin -r -g daos_agent
# No files in a shim package

%changelog
* Mon Sep 23 2024 Kris Jacque <kris.jacque@intel.com> 2.7.100-6
- Bump min supported go version to 1.21

* Thu Aug 15 2024 Michael MacDonald <mjmac@google.com> 2.7.100-5
- Add libdaos_self_test.so to client RPM

Expand All @@ -601,7 +604,7 @@ getent passwd daos_agent >/dev/null || useradd -s /sbin/nologin -r -g daos_agent
* Thu Jul 11 2024 Dalton Bohning <dalton.bohning@intel.com> 2.7.100-3
- Add pciutils-devel build dep for client-tests package

* Thu Jun 24 2024 Tom Nabarro <tom.nabarro@intel.com> 2.7.100-2
* Mon Jun 24 2024 Tom Nabarro <tom.nabarro@intel.com> 2.7.100-2
- Add pciutils runtime dep for daos_server lspci call
- Add pciutils-devel build dep for pciutils CGO bindings

Expand Down
5 changes: 5 additions & 0 deletions utils/scripts/install-ubuntu.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ set -e

arch=$(uname -i)

# DAOS requires newer golang version than the one available in core ubuntu repo
apt-get install gpg-agent software-properties-common
add-apt-repository ppa:longsleep/golang-backports
apt update
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought I'd made a comment on this change but I can't see it anywhere, apologies if I'm repeating myself here.

One of the intentions of these scripts is that an admin should be able to call them to install required packages and to do so safely, so therefore they install packages without the -y option for example. The package manager/repo configuration is all done in the dockerfile/helper scripts and this script then just installs the named packages.

This code breaks that model so the script will re-configure the node as well as install code and in addition it's also possibly not idempotent any more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested moving the Dockerfile Ubuntu version forward to 24.04 LTS and ran into a warning with the interception library (src/client/dfuse/il), I think due to an upgraded version of gcc.

In file included from src/client/dfuse/il/int_posix.c:32:
src/client/dfuse/il/intercept.h:38:21: error: ‘fseek64’ specifies less restrictive attribute than its target ‘fseek’: ‘nonnull’ [-Werror=missing-attributes]
   38 |         ACTION(int, fseek, (FILE *, long, int))                                                    \
      |                     ^~~~~
src/client/dfuse/il/intercept.h:106:27: note: in definition of macro ‘IOIL_DECLARE_ALIAS64’
  106 |         DFUSE_PUBLIC type name##64 params __attribute__((weak, alias(#name)));
      |                           ^~~~
src/client/dfuse/il/int_posix.c:3023:1: note: in expansion of macro ‘FOREACH_ALIASED_INTERCEPT’
 3023 | FOREACH_ALIASED_INTERCEPT(IOIL_DECLARE_ALIAS64)
      | ^~~~~~~~~~~~~~~~~~~~~~~~~
In file included from src/client/dfuse/il/int_posix.c:21:
/usr/include/stdio.h:779:12: note: ‘fseek64’ target declared here
  779 | extern int fseek (FILE *__stream, long int __off, int __whence)
      |            ^~~~~
cc1: all warnings being treated as errors

I think pushing up the Ubuntu version is out of scope for this task, given that it'll require production code changes.

I could shift the PPA addition to the Dockerfile if that would be an acceptable compromise.


apt-get install \
autoconf \
build-essential \
Expand Down
Loading