Thread (3 messages) flat view 3 messages, 2 authors, 2020-01-20

Re: [PATCH v2] ARM: decompressor: simplify libfdt builds

From: Masahiro Yamada <masahiroy@kernel.org>
Date: 2020-01-20 04:30:26
Also in: lkml

Hi Russell,

On Mon, Jan 20, 2020 at 7:48 AM Russell King - ARM Linux admin
[off-list ref] wrote:
On Sun, Jan 19, 2020 at 10:08:22AM +0900, Masahiro Yamada wrote:
quoted
Copying source files during the build time may not end up with
as clean code as expected.

lib/fdt*.c simply wrap scripts/dtc/libfdt/fdt*.c, and it works
nicely. Let's follow that approach for the arm decompressor, too.

Add four wrappers, arch/arm/boot/compressed/fdt*.c and remove
the Makefile messes. Another nice thing is we no longer need to
maintain the own libfdt_env.h because the decompressor can include
<linux/libfdt_env.h>.
Hi,

This is a nice idea, but as Stephen's build has found, it is a very
fragile change, particularly if you're doing a rebuild of an existing
tree.

Stephen's issue appears to be that - he has stale "shipped" copies
that the old Makefile implementation created, which were attempted
to be built with this patch applied.  The result of that is we
try and pick up scripts/dtc/libfdt/libfdt_env.h.

The whole point of the kernel build system is so that we can make
changes to the kernel tree, and then build the kernel, and have the
build system work out how to rebuild the kernel in a proper and safe
way without us having to endlessly clean the build tree just because
a few patches have been added.  This patch breaks that expectation.

At the very least, this build-breaking nature needs to be mentioned,
preferably telling people what they should be doing to fix the issue.

An even better would be to find some way to avoid the issue in the
first place, or find some way to warn about it - maybe by leaving a
libfdt_env.h behind that has an appropriate #warning in it telling
people what to do.  Or something.

Thanks.

Sorry for causing a trouble again.


I will add the following code in arch/arm/boot/compressed/Makefile:

# These were previously generated C files. When you are building the kernel
# with O=, make sure to remove the stale files in the output tree. Otherwise,
# the build system would wrongly compile the stale ones.
ifdef building_out_of_srctree
$(shell rm -f $(addprefix $(obj)/, fdt_rw.c fdt_ro.c fdt_wip.c fdt.c))
endif



Maybe we can remove this code in the future,
but we should keep it long enough.
If the out-of-tree "git bisect" crosses this commit,
the same build error would happen.
People usually do not clean the tree while git-bisecting.



I explained it in the commit description too.



I tested the out-of-tree build
with this patch applied/dropped.


For in-tree build,
"git checkout" seems to nicely overwrite
the stale generated files with the new
check-in files.


I attached the v3 patch.

If it is OK with you,
I will put it in the patch tracker.


Thank you.



-- 
Best Regards
Masahiro Yamada

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help