Skip to content

Change apps/ Makefiles handling of HL_TARGET#5539

Closed
steven-johnson wants to merge 1 commit intomasterfrom
srj/apps-targets
Closed

Change apps/ Makefiles handling of HL_TARGET#5539
steven-johnson wants to merge 1 commit intomasterfrom
srj/apps-targets

Conversation

@steven-johnson
Copy link
Contributor

Always specify a Generator's target value as $HL_TARGET, rather than $*; this allows us more flexibility to cross-compile while adding extra features to HL_TARGET that may not be reflected in the base build tooling.

Always specify a Generator's `target` value as `$HL_TARGET`, rather than `$*`; this allows us more flexibility to cross-compile while adding extra features to HL_TARGET that may not be reflected in the base build tooling.
@steven-johnson steven-johnson added the skip_buildbots Do not run buildbots on this PR. Must add before opening PR as we scan labels immediately. label Dec 9, 2020
@steven-johnson
Copy link
Contributor Author

(skipping buildbots for now since this change is controversial)

@abadams
Copy link
Member

abadams commented Dec 9, 2020

This breaks any workflow that involves rapidly toggling between two different targets (e.g. adding and removing -profile, or -cuda, or -debug). It makes it really easy to have the actual compiled target not match the target implied by the path. The effect is that you "make clean" and rebuild constantly. Moving away from depending on environment variables in the makefiles was a major usability improvement for me, and I'd hate to backslide. I think there are other ways to deal with the cross-compiler issue (I posted one in the other PR).

@steven-johnson
Copy link
Contributor Author

Closing this for now.

@steven-johnson steven-johnson deleted the srj/apps-targets branch February 8, 2021 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip_buildbots Do not run buildbots on this PR. Must add before opening PR as we scan labels immediately.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants