Skip to content
This repository has been archived by the owner on Feb 7, 2023. It is now read-only.

Fail to build sbsigntool #194

Open
yizhao1 opened this issue Mar 10, 2021 · 27 comments
Open

Fail to build sbsigntool #194

yizhao1 opened this issue Mar 10, 2021 · 27 comments

Comments

@yizhao1
Copy link
Contributor

yizhao1 commented Mar 10, 2021

The commit fa5550d introduced a do_configure error:
| configure: error: cannot find the gnu-efi crt path

@yizhao1
Copy link
Contributor Author

yizhao1 commented Mar 10, 2021

@apalos

@apalos
Copy link
Contributor

apalos commented Mar 10, 2021

@yizhao1 any more info on the build? Distro, release etc would be useful. I compiled this on arm/x86 and couldn't reproduce it locally, this is probably coming from the gnu-efi you are using

@yizhao1
Copy link
Contributor Author

yizhao1 commented Mar 10, 2021 via email

@yizhao1
Copy link
Contributor Author

yizhao1 commented Mar 10, 2021

@apalos If the gnu-efi is installed on the host, there is no such issue. But this is not correct, it shouldn't check host gnu-efi.

@apalos
Copy link
Contributor

apalos commented Mar 10, 2021

@apalos If the gnu-efi is installed on the host, there is no such issue. But this is not correct, it shouldn't check host gnu-efi.
@yizhao1 no it shouldn't. That's why my commit added both gnu-efi and gnu-efi-native. Apparently some of the makefiles just look into host directories by default. I'll have a look

@apalos
Copy link
Contributor

apalos commented Mar 10, 2021

@yizhao1 confirmed it, the configure.ac of the upstream sbsigntools hardcodes the search dir for gnu-efi to
/lib /lib64 /usr/lib /usr/lib64 /usr/lib32 /lib/efi /lib64/efi /usr/lib/efi /usr/lib64/efi /usr/lib/gnuefi /usr/lib64/gnuefi
which matches exactly what you saw.

Let me see if I can send a patch making those configurable

@apalos
Copy link
Contributor

apalos commented Mar 10, 2021

@yizhao1 can you give this a shot? Since we explicitly add the correct include directories, removing the check should be fine.

index 4ffb68ffa024..34bb998a2201 100644
--- a/configure.ac
+++ b/configure.ac
@@ -75,9 +75,9 @@ for path in /lib /lib64 /usr/lib /usr/lib64 /usr/lib32 /lib/efi /lib64/efi /usr/
        CRTPATH=$path
     fi
 done
-if test -z "$CRTPATH"; then
-   AC_MSG_ERROR([cannot find the gnu-efi crt path])
-fi
+#if test -z "$CRTPATH"; then
+   #AC_MSG_ERROR([cannot find the gnu-efi crt path])
+#fi
 
 EFI_CPPFLAGS="-I/usr/include/efi -I/usr/include/efi/$EFI_ARCH \
  -DEFI_FUNCTION_WRAPPER"

@apalos
Copy link
Contributor

apalos commented Mar 10, 2021

@yizhao1 even betterm can you apply the following and let me know?
apalos@00835a8

It seemed fine on my arm64/x86 here, but the more the merrier I guess.

@yizhao1
Copy link
Contributor Author

yizhao1 commented Mar 11, 2021

@apalos I have tested your patch but I found there seems something wrong with git.ozlabs.org, I can not pull the ccan git repo. It's not a good idea to download the source during do_configure/do_compile. The users may set BB_NO_NETWORK in their project to disable network access. We'd better to download all sources in do_fetch step. So I think the better way is keep the ccan.git.tar.bz2 and comple it in do_configure. Just like previous recipe did before.

@apalos
Copy link
Contributor

apalos commented Mar 11, 2021

@yizhao1 fair enough, I can probably revert that change.
Any luck up to now to check the current changes?

@yizhao1
Copy link
Contributor Author

yizhao1 commented Mar 12, 2021

@apalos I have tested today. It works for me now. But there is a warning against with poky latest master brach:
WARNING: QA Issue: sbsigntool: native/nativesdk class is not inherited last, this can result in unexpected behaviour. Classes inherited after native/nativesdk: pkgconfig.bbclass siteconfig.bbclass siteinfo.bbclass autotools.bbclass autotools-brokensep.bbclass [native-last]

@coreycothrum
Copy link
Contributor

nothing to add except I'm also seeing this issue, and thanks for looking at it promptly.

@apalos
Copy link
Contributor

apalos commented Mar 12, 2021

@coreycothrum yea apologies for causing it in the first place. I did a clean compile on x86/arm and thought everything was 'fine'. Unfortunately both of those machines had gnu-efi installed, so I completely missed it.

I'll probably be able to post a PR fixing all of them within the day

@apalos
Copy link
Contributor

apalos commented Mar 13, 2021

@coreycothrum @yizhao1 can you check if
#195 fixes the compilation for you?
@yizhao1 BB_NO_NETWORK is a valid point, but there are issues compiling with the old ccan.
If that's ok with you I'll fix that on a follow up commit

@salerio-gs
Copy link

The same commit also given me a very early parse error (from bitbake). Here I am using the head of the poky warrior branch.

ERROR: ExpansionError during parsing /workdir/build/../meta-secure-core/meta-signing-key/recipes-devtools/sbsigntool/sbsigntool_git.bb                                                                     | ETA:  0:01:00
Traceback (most recent call last):
  File "/workdir/poky/bitbake/lib/bb/data_smart.py", line 413, in DataSmart.expandWithRefs(s='${@bb.fetch2.get_srcrev(d)}', varname='SRCPV'):
                     try:
    >                    s = __expand_python_regexp__.sub(varparse.python_sub, s)
                     except SyntaxError as e:
  File "/workdir/poky/bitbake/lib/bb/data_smart.py", line 138, in VariableParse.python_sub(match=<_sre.SRE_Match object; span=(0, 27), match='${@bb.fetch2.get_srcrev(d)}'>):
                         self.contains[k].update(parser.contains[k])
    >            value = utils.better_eval(codeobj, DataContext(self.d), {'d' : self.d})
                 return str(value)
  File "/workdir/poky/bitbake/lib/bb/utils.py", line 423, in better_eval(source=<code object <module> at 0x7fa0ea65fae0, file "Var <SRCPV>", line 1>, locals={'d': <bb.data_smart.DataSmart object at 0x7fa0e9a41160>}, extraglobals={'d': <bb.data_smart.DataSmart object at 0x7fa0e9a41160>}):
                 ctx[g] = extraglobals[g]
    >    return eval(source, ctx, locals)
     
  File "Var <SRCPV>", line 1, in <module>
  File "/workdir/poky/bitbake/lib/bb/fetch2/__init__.py", line 770, in get_srcrev(d=<bb.data_smart.DataSmart object at 0x7fa0e9a41160>, method_name='sortable_revision'):
             raise FetchError("The SRCREV_FORMAT variable must be set when multiple SCMs are used.\n"\
    >                         "The SCMs are:\n%s" % '\n'.join(scms))
     
bb.data_smart.ExpansionError: Failure expanding variable SRCPV, expression was ${@bb.fetch2.get_srcrev(d)} which triggered exception FetchError: Fetcher failure: The SRCREV_FORMAT variable must be set when multiple SCMs are used.
The SCMs are:
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/sbsigntools.git;protocol=https;name=sbsigntool
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/sbsigntools.git;protocol=https

If I back up meta-secure-core one commit earlier, the problem goes away.

@apalos
Copy link
Contributor

apalos commented Mar 13, 2021

@salerio-gs Does the proposed PR solves your problems as well?

@salerio-gs
Copy link

@salerio-gs Does the proposed PR solves your problems as well?

Not tried but I can do shortly

@apalos
Copy link
Contributor

apalos commented Mar 13, 2021

@salerio-gs Does the proposed PR solves your problems as well?

Not tried but I can do shortly

Please do, the SRC_URI changed as well on the PR. It probably fixes your poky builds as well

@salerio-gs
Copy link

salerio-gs commented Mar 13, 2021

@salerio-gs Does the proposed PR solves your problems as well?

Not tried but I can do shortly

Please do, the SRC_URI changed as well on the PR. It probably fixes your poky builds as well

Exactly the same error I'm afraid (fix branch of https://github.com/apalos/meta-secure-core.git).
Don't know if its significant but my machine is intel-corei7-64 (meta-intel warrior branch again).
Its almost as though there is a syntax error in the recipe (or maybe the recipe uses something poky warrior is not happy with)?

@apalos
Copy link
Contributor

apalos commented Mar 13, 2021

@salerio-gs Does the proposed PR solves your problems as well?

Not tried but I can do shortly

Please do, the SRC_URI changed as well on the PR. It probably fixes your poky builds as well

Exactly the same error I'm afraid (fix branch of https://github.com/apalos/meta-secure-core.git).
Don't know if its significant but my machine is intel-corei7-64 (meta-intel warrior branch again).
Its almost as though there is a syntax error in the recipe (or maybe the recipe uses something poky warrior is not happy with)?

The weird this is that the SRC_URI the previous version was complaining about changed from
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/sbsigntools.git;protocol=https;name=sbsigntool
to
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/sbsigntools.git

Can you do a bitbake -c cleanall sbsigntool before rebuilding? (and make sure your SRC_URI is correct)

@salerio-gs
Copy link

salerio-gs commented Mar 13, 2021

@salerio-gs Does the proposed PR solves your problems as well?

Not tried but I can do shortly

Please do, the SRC_URI changed as well on the PR. It probably fixes your poky builds as well

Exactly the same error I'm afraid (fix branch of https://github.com/apalos/meta-secure-core.git).
Don't know if its significant but my machine is intel-corei7-64 (meta-intel warrior branch again).
Its almost as though there is a syntax error in the recipe (or maybe the recipe uses something poky warrior is not happy with)?

The weird this is that the SRC_URI the previous version was complaining about changed from
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/sbsigntools.git;protocol=https;name=sbsigntool
to
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/sbsigntools.git

Can you do a bitbake -c cleanall sbsigntool before rebuilding? (and make sure your SRC_URI is correct)

I understand the issue now. As I noted I use the meta-intel layer. It turns out that this layer has its own sbsigntool-native recipe (https://git.yoctoproject.org/cgit/cgit.cgi/meta-intel/tree/recipes-support/sbsigntool/sbsigntool-native_git.bb?h=warrior) and the error is being cause by you having changed the SRC_URI from that in the meta-intel recipe (I think) which causes bitbake to get very confused. If I BBMASK your modified recipe my build works again. Not sure of the final solution here, but if the build uses meta-intel BSP layer then this will occur.

I was wrong, the above explanation is incorrect. I found a sbsigntool.bbappend in my own layer (which I had totally forgot about).

@apalos
Copy link
Contributor

apalos commented Mar 13, 2021

@salerio-gs Does the proposed PR solves your problems as well?

Not tried but I can do shortly

Please do, the SRC_URI changed as well on the PR. It probably fixes your poky builds as well

Exactly the same error I'm afraid (fix branch of https://github.com/apalos/meta-secure-core.git).
Don't know if its significant but my machine is intel-corei7-64 (meta-intel warrior branch again).
Its almost as though there is a syntax error in the recipe (or maybe the recipe uses something poky warrior is not happy with)?

The weird this is that the SRC_URI the previous version was complaining about changed from
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/sbsigntools.git;protocol=https;name=sbsigntool
to
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/sbsigntools.git
Can you do a bitbake -c cleanall sbsigntool before rebuilding? (and make sure your SRC_URI is correct)

I understand the issue now. As I noted I use the meta-intel layer. It turns out that this layer has its own sbsigntool-native recipe (https://git.yoctoproject.org/cgit/cgit.cgi/meta-intel/tree/recipes-support/sbsigntool/sbsigntool-native_git.bb?h=warrior) and the error is being cause by you having changed the SRC_URI from that in the meta-intel recipe (I think) which causes bitbake to get very confused. If I BBMASK your modified recipe my build works again. Not sure of the final solution here, but if the build uses meta-intel BSP layer then this will occur.

I was wrong, the above explanation is incorrect. I found a sbsigntool.bbappend in my own layer (which I had totally forgot about).

Ok no worries. The intel recipe you pasted fixes the ccan problem though, so I'll update that as well

@apalos
Copy link
Contributor

apalos commented Mar 13, 2021

I just forced pushed the changes with ccan being static as well

@coreycothrum
Copy link
Contributor

@apalos I should get a chance to try this out tomorrow.

I'm not sure if it's an issue, but the inheritance of the native class seems a little outside of best practices.

We've now got both inherit native and BBCLASSEXTEND = "native". According to the docs both aren't needed. Additionally, using inherit native requires the file to be renamed sbsigntool-native.bb (requires the -native.bb suffix) and precludes building for the target.

Does that make sense for this recipe? I don't see any reason to change that aspect from what it was before.

@apalos
Copy link
Contributor

apalos commented Mar 15, 2021

@apalos I should get a chance to try this out tomorrow.

I'm not sure if it's an issue, but the inheritance of the native class seems a little outside of best practices.

We've now got both inherit native and BBCLASSEXTEND = "native". According to the docs both aren't needed. Additionally, using inherit native requires the file to be renamed sbsigntool-native.bb (requires the -native.bb suffix) and precludes building for the target.

Does that make sense for this recipe? I don't see any reason to change that aspect from what it was before.

The previous binary was not running on anything but x86 anyway. So I don't think the non-native build made too much sense to begin with.

@coreycothrum
Copy link
Contributor

@coreycothrum @yizhao1 can you check if
#195 fixes the compilation for you?

Working for me

@yizhao1
Copy link
Contributor Author

yizhao1 commented Mar 18, 2021

@coreycothrum @yizhao1 can you check if
#195 fixes the compilation for you?
@yizhao1 BB_NO_NETWORK is a valid point, but there are issues compiling with the old ccan.
If that's ok with you I'll fix that on a follow up commit

It works well with latest git rev. Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants