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

Add space between -I and path for CPPFLAGS #1474

Closed
wants to merge 2 commits into from

Conversation

roamingthings
Copy link
Contributor

In order for the Eclipse CDT GCC Build Output Parser to be able to
pick up the include paths for C++/cpp files a space is required
between the compiler option (-I) and the actual path.

In order for the Eclipse CDT GCC Build Output Parser to be able to
pick up the include paths for C++/cpp files a space is required
between the compiler option (-I) and the actual path.
@CLAassistant
Copy link

CLAassistant commented Jan 7, 2018

CLA assistant check
All committers have signed the CLA.

@@ -244,12 +244,12 @@ $(1)/%.o: $$(COMPONENT_PATH)/$(1)/%.c $(COMMON_MAKEFILES) $(COMPONENT_MAKEFILE)

$(1)/%.o: $$(COMPONENT_PATH)/$(1)/%.cpp $(COMMON_MAKEFILES) $(COMPONENT_MAKEFILE) | $(COMPONENT_SRCDIRS)
$$(summary) CXX $$(patsubst $$(PWD)/%,%,$$(CURDIR))/$$@
$$(CXX) $$(CXXFLAGS) $$(CPPFLAGS) $$(addprefix -I,$$(COMPONENT_INCLUDES)) $$(addprefix -I,$$(COMPONENT_EXTRA_INCLUDES)) -I$(1) -c $$< -o $$@
$$(CXX) $$(CXXFLAGS) $$(CPPFLAGS) $$(addprefix -I ,$$(COMPONENT_INCLUDES)) $$(addprefix -I,$$(COMPONENT_EXTRA_INCLUDES)) -I$(1) -c $$< -o $$@
Copy link
Member

@igrr igrr Jan 7, 2018

Choose a reason for hiding this comment

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

There are another two -I flags in this line (and the second one), should they be updated as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The part that is responsible for C has already been correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've just reviewed my changes and it seems as if I've missed to places

@igrr
Copy link
Member

igrr commented Jan 7, 2018

Thanks for submitting this, @roamingthings! Do you have a reference why this is only needed for C++ files but not for C?

In order for the Eclipse CDT GCC Build Output Parser to be able to
pick up the include paths for C++/cpp files a space is required
between the compiler option (-I) and the actual path.
@roamingthings
Copy link
Contributor Author

The part that is responsible for C has already been correct.

@projectgus projectgus added the Status: Pending blocked by some other factor label Jan 8, 2018
@projectgus
Copy link
Contributor

Thanks @roamingthings . I've squashed these two commits and cherry-picked into our internal merge queue.

igrr pushed a commit that referenced this pull request Jan 9, 2018
In order for the Eclipse CDT GCC Build Output Parser to be able to
pick up the include paths for C++/cpp files a space is required
between the compiler option (-I) and the actual path.

Merges #1474
igrr pushed a commit that referenced this pull request Jan 9, 2018
@projectgus
Copy link
Contributor

Thanks again @roamingthings . Cherry-picked in 77adf85, and @igrr noticed the component directory also may need a space so this was added in 3f5d6cf.

@projectgus projectgus closed this Jan 9, 2018
@igrr igrr removed the Status: Pending blocked by some other factor label Aug 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants