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

Cu-2zvpafe/create makefile #1

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

ethanckim
Copy link
Collaborator

No description provided.

@ethanckim
Copy link
Collaborator Author

ethanckim commented Nov 12, 2022

After running the makefile, I have ran:
For native compilation (using host gcc): gcc -o hello_host ../src/hello.c
For cross-compilation to RasberryPi ARM: ./gcc-linaro-7.3.1-2018.05-x86_64_arm-linux-gnueabihf/bin/arm-linux-gnueabihf-gcc -o hello_target ../src/hello.c

The output is a runnable executable for both architectures.

I was wondering if I am working towards the correct direction and what the next step would be for the following ticket

@ethanckim ethanckim requested a review from RyanJMah November 12, 2022 18:33
@ethanckim ethanckim changed the title Cu 2zvpafe/create makefile Cu-2zvpafe/create makefile Nov 12, 2022
@Nushaab-Syed Nushaab-Syed added the wip work in progress, still pushing commits label Nov 24, 2022
Copy link

@RyanJMah RyanJMah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this took so long, overall looks fine. I left some comments that I urge you to read before merging though. I'll leave it up to you to merge when you feel it's ready.

.PHONY: install-toolchain host target clean

install-toolchain:
$(MKDIR) ./downloads

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we consider moving this downloads folder into ./build? I'm not sure I like it in the main directory like this.

Also, consider making variables defined near the top of the file for these directories. For example:

BUILD_DIR = ./build

$(CC) -o ./install/$@ $^ $(CFLAGS)

clean:
$(RM) $(@D)/build

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think make clean should delete the toolchain. Maybe we can have a clean-toolchain target or something like that.

@echo $(PATH)

target: $(C_SOURCES)
$(MKDIR) install

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like calling this directory install. I think we should put all of our compiled objects into a subdirectory of ./build


target: $(C_SOURCES)
$(MKDIR) install
$(CC-arm) -o ./install/$@ $^ $(CFLAGS)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should work for now, but the typical way to do this is to compile each .c file without linking (the -c flag), then link afterwards as a separate step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wip work in progress, still pushing commits
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants