Build-Linux#64
Conversation
jaykrell
force-pushed
the
linux
branch
8 times, most recently
from
90cc34b to
d7509f9
Compare
jaykrell
force-pushed
the
linux
branch
2 times, most recently
from
b8e006f to
9f60ecb
Compare
mebersol
left a comment
mebersol
left a comment
There was a problem hiding this comment.
[Copilot] Linux build support is a welcome addition. A few items to address before merge:\n\n- Workflow: Outdated action version (checkout@v3 → v4), potentially wrong GCC prefix (underscores vs hyphens), heavy mono-complete dependency that may not be needed for CLANGPDB, and no pip/submodule caching.\n- DSC: ASLCC_FLAGS workaround is reasonable but fragile — should track as tech debt.\n- FDF: Align=Auto change is sound but differs from AARCH64's Align=4K approach — worth confirming the inconsistency is intentional.
| [Rule.Common.SEC] | ||
| FILE SEC = $(NAMED_GUID) { | ||
| PE32 PE32 $(INF_OUTPUT)/$(MODULE_NAME).efi | ||
| PE32 PE32 Align=Auto $(INF_OUTPUT)/$(MODULE_NAME).efi |
There was a problem hiding this comment.
[Copilot] Changing from Align=32 to Align=Auto makes sense for cross-toolchain compatibility. Note that the AARCH64 FDF uses Align=4K for its SEC/PEI_CORE/PEIM rules rather than Align=Auto — is the difference intentional? Also, the DXE_DRIVER/DXE_RUNTIME_DRIVER/UEFI_APPLICATION rules in this file don't have explicit alignment — worth confirming they don't need the same treatment for Linux builds.
There was a problem hiding this comment.
This is all accidental and not full investigated, agreed.
There was a problem hiding this comment.
I suggest Auto everywhere. I suggest it shouldn't even be an option.
Users should be expected to set FileAlign either to match SectionAlign for execute-in-place or simplicity, or small if they desire packing.
There was a problem hiding this comment.
This is what Copilot suggested to Chris though. So this is Copilot reviewing itself and not agreeing with itself. I am just a human bystander.
There was a problem hiding this comment.
Let's not commit until this is looked at more. Sorry Chris.
Becauses, as Copilot points out, there's a gazillion of these. Why change only a few?
Let's look at the file alignments and placement in fv/fd before/after.
I mean, you know, anytime we produce PE directly with mslink or ldlink, we have two alignments to specify which should be plenty to make it correct. Gcc via ELF is another matter but not relevant here (for gcc we should give up on native gcc and instead use mingw-gcc, so much cleaner..even if we have to build it ourselves though I don't think we'll have to...)
There was a problem hiding this comment.
Alternatively we can undo this part and commit. So it compiles and links but doesn't run. Meh.
There was a problem hiding this comment.
is it because align=auto is only being done for these modules which are execute in place (XIP)? I wonder if the alignment from codegen is different for these which is why we need align auto?
|
The open question on the gcc stuff, is why is it working in closed source? |
4 participants
This PR adds support fo building with clangpdb on Linux.
It also adds gcc, but that is disabled due to an error.
The results might not yet boot, that is a secondary concern to be dealt with, later.
Also fixes gcc flags. Also aligns images in FV -- maybe wrong linker flags?