Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Makefile does not recompile despite dependent header file changes on MSYS2 #528

Open
jlj-ee opened this issue Mar 29, 2023 · 8 comments · Fixed by #653
Open

Makefile does not recompile despite dependent header file changes on MSYS2 #528

jlj-ee opened this issue Mar 29, 2023 · 8 comments · Fixed by #653
Assignees
Labels
build system This issue or pull request is related to the MSDK build system

Comments

@jlj-ee
Copy link
Contributor

jlj-ee commented Mar 29, 2023

I am not seeing recompilation happen if I edit a header file and then recompile.
Minimal example to repro using VS Code hello_world example.

  1. Clean and build the hello_world project. Observe the build task terminal output:
Loaded project.mk
  CC    main.c
  CC    C:/MaximSDK/Libraries/Boards/MAX32670/EvKit_V1/Source/board.c
  CC    C:/MaximSDK/Libraries/Boards/MAX32670/EvKit_V1/../Source/stdio.c
  CC    C:/MaximSDK/Libraries/Boards/MAX32670/EvKit_V1/../Source/led.c
  CC    C:/MaximSDK/Libraries/Boards/MAX32670/EvKit_V1/../Source/pb.c
  AS    C:/MaximSDK/Libraries/CMSIS/Device/Maxim/MAX32670/Source/GCC/startup_max32670.S
  CC    C:/MaximSDK/Libraries/Boards/MAX32670/EvKit_V1/Source/rom_stub.c
  CC    C:/MaximSDK/Libraries/CMSIS/Device/Maxim/MAX32670/Source/heap.c
  CC    C:/MaximSDK/Libraries/CMSIS/Device/Maxim/MAX32670/Source/system_max32670.c
  LD    <mydir>/examples/max32670_hello_world/build/max32670_hello_world.elf 
arm-none-eabi-size --format=berkeley <mydir>/examples/max32670_hello_world/build/max32670_hello_world.elf
   text    data     bss     dec     hex filename
  35676    2500     628   38804    9794 <mydir>/examples/max32670_hello_world/build/max32670_hello_world.elf
  1. Open /Libraries/Boards/MAX32670/EvKit_V1/Include/board.h and change CONSOLE_BAUD to some other value, e.g. 9600.
  2. Build again (without cleaning). Observe that no compilation occurs in the task terminal output:
Loaded project.mk
arm-none-eabi-size --format=berkeley <mydir>/examples/max32670_hello_world/build/max32670_hello_world.elf 
   text    data     bss     dec     hex filename
  35676    2500     628   38804    9794 <mydir>/examples/max32670_hello_world/build/max32670_hello_world.elf 
  1. Confirm via serial terminal that baud is still the original value, not the new value.

The same behavior is seen if, instead of modifying board.h, I include a new main.h file in my copy of the example and override the CONSOLE_BAUD there.

I looked into tweaking gcc.mk to address this (e.g. using an approach like this) but I could not get it to work due to lack of experience with GNU-make. I do see that the *.d dependencies are already being generated as part of the existing MSDK build approach, but it still appears as though changes to (some?) headers do not prompt recompilation via the makefile like I would have expected.

@Jake-Carter Jake-Carter added the build system This issue or pull request is related to the MSDK build system label Mar 31, 2023
@Jake-Carter
Copy link
Contributor

Thanks @jlj-ee, I think we're missing the inclusion of the dependency files as prerequisites for our implicit rules. I'll take a look at this

@Jake-Carter Jake-Carter self-assigned this Mar 31, 2023
@Jake-Carter
Copy link
Contributor

Quick update on this ticket: The dependency files seem to be working as expected with our current gcc.mk on Linux, but are failing to work on Windows.

It's getting trickier to track down the root cause of this... suspected path parsing issues on Windows, but still trying to identify the root cause.

@jlj-ee
Copy link
Contributor Author

jlj-ee commented Apr 20, 2023

Thanks @Jake-Carter for the update. I am on Windows so that syncs with my experience. In the meantime I just clean before I build to be sure I get the latest.

Is this the implicit rule to include the dependencies?

# Include the automatically generated dependency files.
ifneq (${MAKECMDGOALS},clean)
-include ${wildcard ${BUILD_DIR}/*.d} __dummy__
endif

When I went to read about auto-dependency generation, I came across this article that uses a slightly different approach and includes an explicit rule to indicate that the object files should depend on both the source files and generated dependencies:

DEPDIR := .deps
DEPFLAGS = -MT $@ -MMD -MP -MF $(DEPDIR)/$*.d

COMPILE.c = $(CC) $(DEPFLAGS) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c

%.o : %.c
%.o : %.c $(DEPDIR)/%.d | $(DEPDIR)
        $(COMPILE.c) $(OUTPUT_OPTION) $<

$(DEPDIR): ; @mkdir -p $@

DEPFILES := $(SRCS:%.c=$(DEPDIR)/%.d)
$(DEPFILES):
include $(wildcard $(DEPFILES))

I do not see anything similar in the explicit rule for MSDK:

${BUILD_DIR}/%.o: %.c
	@if [ 'x${ECLIPSE}' != x ];                                                                                 \
	then                                                                                                        \
		echo ${CC} ${CFLAGS} -o $(call fixpath,${@}) $(call fixpath,${<}) | sed 's/-I\/\(.\)\//-I\1:\//g' ; \
	elif [ 'x${VERBOSE}' != x ];                                                                                \
	then                                                                                                        \
	    echo ${CC} ${CFLAGS} -o $(call fixpath,${@}) $(call fixpath,${<});                                      \
	else                                                                                                        \
	    echo "  CC    ${<}";                                                                                    \
	fi
	@${CC} ${CFLAGS} -o $(call fixpath,${@}) $(call fixpath,${<})
ifeq "$(CYGWIN)" "True"
	@sed -i -r -e 's/([A-Na-n]):/\/cygdrive\/\L\1/g' -e 's/\\([A-Za-z])/\/\1/g' ${@:.o=.d}
endif

This doesn't explain why it would work on Linux but not Windows, but would there be a benefit to adding the .d files as a dependency in the explicit rule, as in the linked article?
Apologies if I am off-base; while I'm working on coming up the learning curve, my knowledge of make systems is still very rudimentary.

@Jake-Carter
Copy link
Contributor

Yes, -include ${wildcard ${BUILD_DIR}/*.d} __dummy__ is how the dependency files are included. It's a slightly different approach than the article you linked, but I have been referencing that article as I've been troubleshooting. It's a really great implementation so thanks for sharing.

Since the existing setup works on Linux I initially guessed it was a path issue on Windows. In the dependency rules themselves I could see some malformed paths being generated like the ones below...

C:/Users/JCarter3/repos/fork/msdk/Examples/MAX78000/Hello_World/build/board.o: \
 # ...
 C:/Users/JCarter3/repos/fork/msdk/Libraries/PeriphDrivers/Include/MAX78000/gpio.h \
 C:/Users/JCarter3/repos/fork/msdk/Libraries/PeriphDrivers/Include/MAX78000/mxc_pins.h \
 c:\users\jcarter3\repos\fork\msdk\libraries\miscdrivers\led\led.h \  #<-- malformed
 c:\users\jcarter3\repos\fork\msdk\libraries\miscdrivers\pushbutton\pb.h \ #<-- malformed
 C:/Users/JCarter3/repos/fork/msdk/Libraries/CMSIS/Device/Maxim/MAX78000/Include/simo_regs.h \
 c:\users\jcarter3\repos\fork\msdk\libraries\miscdrivers\display\tft_ssd2119.h \ #<-- malformed
 C:/Users/JCarter3/repos/fork/msdk/Libraries/PeriphDrivers/Include/MAX78000/spi.h \
 C:/Users/JCarter3/repos/fork/msdk/Libraries/CMSIS/Device/Maxim/MAX78000/Include/spi_regs.h \
 C:/Users/JCarter3/repos/fork/msdk/Libraries/PeriphDrivers/Include/MAX78000/mxc_assert.h \
 C:/Users/JCarter3/repos/fork/msdk/Libraries/PeriphDrivers/Include/MAX78000/mxc_lock.h \
 c:\users\jcarter3\repos\fork\msdk\libraries\miscdrivers\touchscreen\tsc2046.h #<-- malformed

I thought the forced lower-case paths like c:\users\jcarter3\repos\fork\msdk\libraries\miscdrivers\led\led.h were causing the problems. It turns out if we tell GCC to resolve a relative path the -MD output for it will have this form, and these were coming from our BSP makefiles. I fixed that and got everything to parse correctly, but the behavior is still the same.

C:/Users/JCarter3/repos/fork/msdk/Examples/MAX78000/Hello_World/build/board.o: \
 # ...
 C:/Users/JCarter3/repos/fork/msdk/Libraries/PeriphDrivers/Include/MAX78000/gpio.h \
 C:/Users/JCarter3/repos/fork/msdk/Libraries/PeriphDrivers/Include/MAX78000/mxc_pins.h \
 C:/Users/JCarter3/repos/fork/msdk/Libraries/MiscDrivers/LED/led.h \
 C:/Users/JCarter3/repos/fork/msdk/Libraries/MiscDrivers/PushButton/pb.h \
 C:/Users/JCarter3/repos/fork/msdk/Libraries/CMSIS/Device/Maxim/MAX78000/Include/simo_regs.h \
 C:/Users/JCarter3/repos/fork/msdk/Libraries/MiscDrivers/Display/tft_ssd2119.h \
 C:/Users/JCarter3/repos/fork/msdk/Libraries/PeriphDrivers/Include/MAX78000/spi.h \
 C:/Users/JCarter3/repos/fork/msdk/Libraries/CMSIS/Device/Maxim/MAX78000/Include/spi_regs.h \
 C:/Users/JCarter3/repos/fork/msdk/Libraries/PeriphDrivers/Include/MAX78000/mxc_assert.h \
 C:/Users/JCarter3/repos/fork/msdk/Libraries/PeriphDrivers/Include/MAX78000/mxc_lock.h \
 C:/Users/JCarter3/repos/fork/msdk/Libraries/MiscDrivers/Touchscreen/tsc2046.h

So now I'm porting the solution in the article you shared to our system. It's at a lower priority than some of the other work for the SDK, but hopefully this week I will have it tested. The ordering and approach from the article is slightly different, so it may reveal something we're missing in our setup. I'm still not sure why Windows specifically is failing, but MSYS2 is another factor to consider. I'll keep you posted!

@Jake-Carter
Copy link
Contributor

@jlj-ee FYI #528 seems to (partially) resolve this issue. The behavior is as expected on native windows as of that PR.

The problem seems to be the MSYS2 translation layer for path conversions. Still working on the right mix of /c/ and C:/-style paths inside of the .d files to get MSYS2 builds to behave

@jlj-ee
Copy link
Contributor Author

jlj-ee commented Oct 25, 2023

Hi @Jake-Carter I have updated to the October release but still find that the build system does not detect changes if I only modify a dependent header file. Is there anything I need to change in my makefile or project.mk in order to benefit from the changes you linked a couple months ago that apparently resolved this issue on Windows?

@Jake-Carter
Copy link
Contributor

Hi @jlj-ee

This is fixed for native Windows builds, which is an opt-in feature for this latest release. See the release notes for the Oct release: https://github.com/Analog-Devices-MSDK/msdk/releases/tag/v2023_10

I tagged this ticket to the PR in the comment above and didn't realize it auto-closed. I did not get the paths to work properly on MSYS2...

@Jake-Carter Jake-Carter reopened this Oct 25, 2023
@Jake-Carter Jake-Carter changed the title Makefile does not recompile despite dependent header file changes Makefile does not recompile despite dependent header file changes on MSYS2 Oct 25, 2023
@jlj-ee
Copy link
Contributor Author

jlj-ee commented Oct 25, 2023

Thanks for the clarification @Jake-Carter, I’ll rtfm and try again :)

Edit: Followed the instructions in the release notes to configure for using native Windows make and it works a treat. Thanks, this is a big QoL improvement!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build system This issue or pull request is related to the MSDK build system
Projects
None yet
2 participants