Open bryteise opened 11 months ago
Suggest moving export DEFINES
to execute unconditionally at the end of this file, just before the vim:filetype
line.
The current PR exports all DEFINES (including ones added after the export line, as can be verified by defining one of these values and looking at the full build output), but only when OVERRIDE_SECURITY_POLICY is defined. This does not seem quite correct, the suggested change exports all DEFINES to all subsidiary builds always, which also works and seems more sane/consistent.
@ mikebeaton Thanks for your reminder.
Suggest moving export DEFINES to execute unconditionally at the end of this file, just before the vim:filetype line.
Yes, let's move export DEFINES
out of conditional case but omit the DEFINES
. That is to say export
one word is good enough to export all variables by default. Also it is not necessary to relocate export
at the end of this file to wait for all DEFINES+=
. I suggest we put export in the line ahead of the first DEFINES in Make.defaults.
@dennis-tseng99 I tested the export on its own line before the DEFINES and the build hangs, debugging with --debug=all shows a lot of not recursively expanding
lines so I don't think exporting like that is going to work.
@bryteise Sorry, I cannot reproduce your 'hangs' issue. In my site, build is always successful. For example:
make clean; make RELEASE=0 SHIMSTEM=shim MMSTEM=MokManager FBSTEM=fbx64 ENABLE_HTTPBOOT=1 OVERRIDE_SECURITY_POLICY=1 DEFAULT_LOADER="\\\\\\\\grub.efi" shim.efi.debug shim.efi MokManager.efi fbx64.efi
@dennis-tseng99 Hrm I'm using GNU make 4.4.1 and using the current master. If I add export
between the BOOTCSVNAME
and DEFINES
make just seems to get stuck in a recursive loop (even just running make clean
).
I can replicate the issue under an archlinux container with binutils, gcc, make and git installed (you don't need binutils or gcc but you'll see errors related to them missing repeating when you run make clean
otherwise) after a fresh git clone --recurse-submodules
.
I can confirm the error with export
only, on a Docker archlinux container, and can also confirm there is no error in current Ubuntu or Fedora using just export
; export DEFINES
works in all those cases.
Update: Current Fedora container on Docker (which is indeed make 4.4) does show the same issue.
The location shown in @dennis-tseng99's image is not the first DEFINES
in Make.defaults.
Just export
is wrong. In addition to hanging on make 4.4, it generates the following errors (several lines of each) in make clean
output with make 4.3 which are not present when using export DEFINES
:
clang-15: error: unknown argument: '-maccumulate-outgoing-args
(Fedora)make[1]: clang: No such file or directory
(Ubuntu)(although unlike in 4.4 make clean
does finish).
I've checked all the DEFINES
produced in Make.defaults
and checked all the libraries which they are passed to if they are exported (i.e. Cryptlib
, Cryptlib/OpenSSL
and lib
), and the only define which is really used in any of the libraries is OVERRIDE_SECURITY_POLICY in lib/security_policy.c
.
Several others (i.e. DEFAULT_LOADER
, DEFAULT_LOADER_CHAR
, EFI_ARCH
, DEBUGDIR
, VENDOR_DB_FILE
, VENDOR_DBX_FILE
, VENDOR_CERT_FILE
) are processed by shim.h
, to produce default values if they are missing, and shim.h
is included by all sources in lib/*.c
- but the values produced by this are not themselves used in any lib/*.c
files.
The upshot being, a possibly more correct fix would be to specifically process OVERRIDE_SECURITY_POLICY in lib/Makefile
:
@mikebeaton To consider the code expanding in the near future, I still suggest we should make use of "export DEFINES" in Make.defaults to pass down variable(s) to sub-make. Besides, although shim.h is already included in lib/*.c, when user press make CLI command like make VENDOR_DB_FILE=filename OVERRIDE_SECURITY_POLICY=1
, we still need to pass down VENDOR_DB_FILE=filename
to shim.h by export DEFINES
. Your codes only focus on OVERRIDE_SECURITY_POLICY
compiling problem.
shim.h:
#if defined(VENDOR_DB_FILE)
# define Dennis //for test
# define vendor_authorized vendor_db
# define vendor_authorized_size vendor_db_size
# define vendor_authorized_category VENDOR_ADDEND_DB
#elif defined(VENDOR_CERT_FILE)
# define vendor_authorized vendor_cert
# define vendor_authorized_size vendor_cert_size
# define vendor_authorized_category VENDOR_ADDEND_X509
#else
# define vendor_authorized vendor_null
# define vendor_authorized_size vendor_null_size
# define vendor_authorized_category VENDOR_ADDEND_NONE
#endif
@bryteise Yes, you are right. I can reproduce your hanging problem now. This is because export directive is only used for old GNU make, e.g. 4.2.1. Let's go back to export DEFINES
Okay, that also sounds valid to me - thanks.
I'll close my alternative suggestion.
@bryteise Would you please re-submit your PR which is moving the export DEFINES
to anywhere but not inside a conditional
, just like Mikebeaton's suggestion ? For example, we can move it to the end of Make.defaults. Many thanks.
@dennis-tseng99 Updated, I think that's what you are looking for.
Fixes #596 for me.