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

gmake2: Remove pch from FORCE_INCLUDE to allow NoPCH per-file #1100

Merged
merged 2 commits into from
Jun 3, 2018

Conversation

tdesveauxPKFX
Copy link
Contributor

No description provided.

Copy link
Member

@samsinsane samsinsane left a comment

Choose a reason for hiding this comment

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

Could use some unit tests, kind of surprised that this didn't break anything.

end

if path.iscfile(file.name) then
return 'ALL_CFLAGS'
if fcfg and fcfg.flagsVariable then
Copy link
Member

Choose a reason for hiding this comment

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

I'm not entirely sure what flagsVariable is, why that would override the $(ALL_*FLAGS) flag, or how it's related to PCH files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the per-file arguments variable name.

With gmake2, if files have specific arguments, the generator will create a Makefile var PERFILE_FLAGS_* to group them.

As an example, You can have this:
PERFILE_FLAGS_0 = $(ALL_CXXFLAGS) -mf16c

then your file rule will be
$(SILENT) $(CXX) $(PERFILE_FLAGS_0) $(FORCE_INCLUDE) -o "$@" -MF "$(@:%.o=%.d)" -c "$<"
instead of
$(SILENT) $(CXX) $(ALL_CXXFLAGS) $(FORCE_INCLUDE) -o "$@" -MF "$(@:%.o=%.d)" -c "$<"

The way it is implemented is that ALL_*FLAGS is contained in PERFILE_FLAGS_* if necessary so it doesn't need to be added as well.

Copy link
Member

Choose a reason for hiding this comment

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

I do remember seeing those, this makes sense - thanks for that!

@tdesveauxPKFX
Copy link
Contributor Author

Yeah, completely forgot unit tests. I will implement some later today.

@samsinsane samsinsane merged commit 313d7ed into premake:master Jun 3, 2018
@tdesveauxPKFX tdesveauxPKFX deleted the gmake2/fix-perfile-nopch branch June 3, 2018 14:44
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.

2 participants