From mboxrd@z Thu Jan 1 00:00:00 1970 X-GM-THRID: 6768877611266342912 X-Received: by 2002:a2e:7d01:: with SMTP id y1mr5356268ljc.100.1576143998675; Thu, 12 Dec 2019 01:46:38 -0800 (PST) X-BeenThere: isar-users@googlegroups.com Received: by 2002:ac2:4acf:: with SMTP id m15ls470659lfp.13.gmail; Thu, 12 Dec 2019 01:46:38 -0800 (PST) X-Google-Smtp-Source: APXvYqyvyWpZL0FwESaPth558VWzrNeovXEDV7giNpNe5br7cFk2vvlWcE2QtY2pV4Nyyg4k118C X-Received: by 2002:ac2:59dd:: with SMTP id x29mr4930814lfn.95.1576143998126; Thu, 12 Dec 2019 01:46:38 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1576143998; cv=none; d=google.com; s=arc-20160816; b=jFTs1XFwCch4vdkRLruS3LNEi5eubimTgaXmYMXhMPWvFivdWDEw3C4FCH33ZEuKSw qDohGJqJikEMivZvHk7wCCU8D9exEJq8GZF8rmfgDnvINe9MxMCpDZWbCivSPbDa5goc WEjpYILvXo8HoaF2JnT2xsILfkwcyLoT0wA8SF8+sSdD7Un+8C8lrwFcQWuq7pxnvSHg sK58I3/tqkG5fQE7svCSj4FUFn5gDc2Pi0o+zPk0esSUwWoqjnz0MiGir1PPpiRklypd WFswUfKzoLWgktgYNTzVRhuyy4E2bXISbiWXHmQII0dR4hgyY5nUfOOqTP5HZ/Tw/JOS XppA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject; bh=9qZ/ikk2jqticMgEsStdFzDAH99moM+hLQokWW4Qkxc=; b=PtV+Z9ebMM0QhxviDQzl2CROIAnqkISsBxeZx55mhvc+q3FuZsGUbTCzM6PYnwgu96 ZMnuiIzCSB07KktN/hlZ5suV5P8frTGhyMzbgeCH8+6nqlhq02sAi9ZlFG82a9ryOqWF GtYwURDG+tmSveLflIrI0v5PhRmKAbKdNY6bVxpp6ib/HSWRDEHfnX7keUzT1eNszQxv AJIsSQouz+prZHEU8ITxc8BUA7ejZuFuQcUde0024hD+F3ERHzOzhhIsph6LQ+cZcawP hvCbKRsy2jNIH6jOKYo6PuxZC6L/azYvAzedhuh8umG0SgU3vVXwZX1yliHGrJ1vpirO gZSA== ARC-Authentication-Results: i=1; gmr-mx.google.com; spf=pass (google.com: domain of quirin.gylstorff@siemens.com designates 194.138.37.39 as permitted sender) smtp.mailfrom=quirin.gylstorff@siemens.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=siemens.com Return-Path: Received: from lizzard.sbs.de (lizzard.sbs.de. [194.138.37.39]) by gmr-mx.google.com with ESMTPS id e3si49657ljg.2.2019.12.12.01.46.37 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 12 Dec 2019 01:46:38 -0800 (PST) Received-SPF: pass (google.com: domain of quirin.gylstorff@siemens.com designates 194.138.37.39 as permitted sender) client-ip=194.138.37.39; Authentication-Results: gmr-mx.google.com; spf=pass (google.com: domain of quirin.gylstorff@siemens.com designates 194.138.37.39 as permitted sender) smtp.mailfrom=quirin.gylstorff@siemens.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=siemens.com Received: from mail2.sbs.de (mail2.sbs.de [192.129.41.66]) by lizzard.sbs.de (8.15.2/8.15.2) with ESMTPS id xBC9kbkf013227 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 12 Dec 2019 10:46:37 +0100 Received: from [139.25.69.120] ([139.25.69.120]) by mail2.sbs.de (8.15.2/8.15.2) with ESMTP id xBC9kbc2012026; Thu, 12 Dec 2019 10:46:37 +0100 Subject: Re: [PATCH v5 4/5] linux-custom: rewrite to no longer depend on the kernel's builddeb To: Jan Kiszka , Henning Schild Cc: isar-users@googlegroups.com, Cedric Hombourger References: <79f99cd5-208c-9968-3b78-08234a206e71@siemens.com> <20191211230919.2cf24212@md1za8fc.ad001.siemens.net> <398c47ca-7e4b-5f4e-2acd-aaf9cd6c3f82@siemens.com> From: Gylstorff Quirin Message-ID: Date: Thu, 12 Dec 2019 10:46:29 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: <398c47ca-7e4b-5f4e-2acd-aaf9cd6c3f82@siemens.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-TUID: zllGrrxiKtEV On 12/12/19 9:01 AM, Jan Kiszka wrote: > On 12.12.19 08:57, Gylstorff Quirin wrote: >> >> >> On 12/11/19 11:09 PM, Henning Schild wrote: >>> On Wed, 11 Dec 2019 19:36:05 +0100 >>> Jan Kiszka wrote: >>> >>>> On 11.12.19 16:43, [ext] Jan Kiszka wrote: >>>>> On 11.12.19 16:20, Gylstorff Quirin wrote: >>>>>>> +do_build() { >>>>>>> + >>>>>>> +    # Print a few things that are of particular interest >>>>>>> +    print_settings >>>>>>> + >>>>>>> +    # Process existing kernel configuration to make sure it is >>>>>>> complete >>>>>>> +    # (use defaults for options that were not specified) >>>>>>> +    ${MAKE} O=${KERNEL_BUILD_DIR} olddefconfig prepare || exit >>>>>>> ${?} + >>>>>>> +    # Check if the recipe's PV makes sense >>>>>>> +    KR=$(${MAKE} O=${KERNEL_BUILD_DIR} -s --no-print-directory >>>>>>> kernelrelease) >>>>>>> +    eval $(grep ^CONFIG_LOCALVERSION= >>>>>>> ${KERNEL_BUILD_DIR}/${KCONF} || true) >>>>>>> +    if [ "${PV}-${KERNEL_LOCALVERSION}" != "${KR}" ]; then >>>>>>> +        echo "ERROR: Recipe version >>>>>>> (${PV}-${KERNEL_LOCALVERSION}) does not seem to match the >>>>>>> kernelrelease (${KR})!" 1>&2 >>>>>>> +        echo "ERROR: Make sure the kernel version in your >>>>>>> NAME/PV/PR settings and/or CONFIG_LOCALVERSION are aligned" 1>&2 >>>>>>> +        exit 1 > +    fi >>>>>> >>>>>> we have some CI use case where we build the latest git release >>>>>> could we add something like this >>>>>> >>>>>> -    if [ "${PV}-${KERNEL_LOCALVERSION}" != "${KR}" ]; then >>>>>> -        echo "ERROR: Recipe version >>>>>> (${PV}-${KERNEL_LOCALVERSION}) does not seem to match the >>>>>> kernelrelease (${KR})!" 1>&2 >>>>>> -        echo "ERROR: Make sure the kernel version in your >>>>>> NAME/PV/PR settings and/or CONFIG_LOCALVERSION are aligned" 1>&2 >>>>>> -        exit 1 >>>>>> +    if [ "${PV}" =~ "latest" ]; then >>>>> >>>>> I suspect you wanted to suggest != "latest". >>>>> >>>>>> +        if [ "${PV}-${KERNEL_LOCALVERSION}" != "${KR}" ]; then >>>>>> +            echo "ERROR: Recipe version >>>>>> (${PV}-${KERNEL_LOCALVERSION}) does not seem to match the >>>>>> kernelrelease (${KR})!" 1>&2 >>>>>> +            echo "ERROR: Make sure the kernel version in your >>>>>> NAME/PV/PR settings and/or CONFIG_LOCALVERSION are aligned" 1>&2 >>>>>> +            exit 1 >>>>>> +        fi >>>>> >>>>> We need some relaxation path for the check, yes. Given the other >>>>> versioning issue, I'm still trying to build a complete picture. >>>> >>>> Looking the Henning's commit that introduced the check, it reads to me >>>> like just addressing constraints of the old build approach. The new >>>> one has a way to set LOCALVERSION from the recipe. >>> >>> Yes, the check is just early catching a weird error that would have >>> popped up later. That must have been either the build or the step >>> copying the kernel binary to DEPLOY. >>> >>> If a new way of building can deal with it, the check can be dropped. >>> >>>> So, what we would rather need than this hard check is the following: >>>> >>>>   - optional KERNEL_LOCALVERSION >>>>   - pick-up of LOCALVERSION from the kernel config for the case it was >>>>     defined via the config >>>>   - KERNEL_LOCALVERSION ?= "" to avoid breaking existing users >>>>     needlessly >>>> >>>> That approach would both enable CONFIG_LOCALVERSION usage via own >>>> configs as well as convenient management in recipes via >>>> KERNEL_LOCALVERSION. But it has a catch: We need the LOCALVERSION >>>> information already for the templating step while >>>> dpkg_configure_kernel is part of the build. >>>> >>>> So we may be left with these options: >>>> >>>>   - check if CONFIG_LOCALVERSION == KERNEL_LOCALVERSION, which is true >>>>     when KERNEL_LOCALVERSION is used but could be violated when the >>>>     custom config provides a LOCALVERSION while KERNEL_LOCALVERSION is >>>>     empty >>>>   - always override CONFIG_LOCALVERSION with KERNEL_LOCALVERSION, as in >>>>     this version of the patch - may cause surprises, though >>>>   - try to pick up CONFIG_LOCALVERSION early, but only from a user- >>>>     provided defconfig, not from fragments or templates - maybe too >>>>     unintuitive >>>> >>>> Not so easy. Thoughts? >>> >>> I am not sure i fully get the suggestion. I think you suggest to have a >>> bitbake variable control parts of the config ... that one localversion >>> key in it. >>> >>> The user expectation would probably be that the PV will become _the_ >>> version. So i would go for a sanity check for that, and a warning if >>> not. After that we can discuss a magic that will turn something behind >>> the first or last "-" in PV into CONFIG_LOCALVERSION and patch that >>> into the config. >> You mean a warning or an error? The current version aborts the build, if >> the versions do not match. >> >> As mentioned before: If a mainline kernel is used PV == KERNEL_RELEASE >> is already not fulfilled. So we already have a two expectation the >> Debian user and the bitbake user. >> >>> >>> So instead of a new variable, come up with a new recipe naming >>> convention. And for people that really want to call the recipe >>> "kernel.bb" they would get the default >>> >>> PV = "1.0" >>> PR = "" >>> PLOCALV = "" >>> >>> Would have to check if "PR" is the thing after the first "-" ... But >>> maybe PR is what we are looking for ... >> PR is the revision of the recipe which comes after the first "-". Yocto >> uses its own variable "LINUX_VERSION_EXTENSION" which sets >> CONFIG_LOCALVERSION. > > Then we should do s/KERNEL_LOCALVERSION/LINUX_VERSION_EXTENSION. > > Just leaves us with the other policy questions. > > Jan > From my understanding after reading the patches the KERNEL_LOCALVERSION does not affect the build or installation of the Kernel artifacts as the mandatory control and changelog elements use only KERNEL_NAME_PROVIDED + CHANGELOG_V (PV+PR). So the only occurrence of the LOCALVERSION is in uname and the Package description. If this is the case then the check KERNEL_LOCALVERSION == CONFIG_LOCALVERSION and a warning should be enough. Did I miss something? Quirin