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

CMake: Add ESP_PLATFORM to build environment. #2601

Closed
wants to merge 3 commits into from
Closed

CMake: Add ESP_PLATFORM to build environment. #2601

wants to merge 3 commits into from

Conversation

PerMalmberg
Copy link
Contributor

As first discussed in this forum thread, the ESP_PLATFORM define is missing during configuration and build when building with CMake.

@PerMalmberg PerMalmberg changed the title Add ESP_PLATFORM to build environment. CMake: Add ESP_PLATFORM to build environment. Oct 21, 2018
@PerMalmberg
Copy link
Contributor Author

Just realized that GitHub updates pull requests when new changes are added to the same branch, sorry about that. Both commits are however related to the same forum thread.

PerMalmberg added a commit to PerMalmberg/Smooth that referenced this pull request Oct 23, 2018
@igrr igrr requested a review from projectgus November 23, 2018 06:34
@PerMalmberg
Copy link
Contributor Author

Just merged upstream/master into my fork to resolve the conflict.

@projectgus
Copy link
Contributor

Hi @PerMalmberg ,

Thanks for submitting this, sorry for the extended delay in getting back to you.

This looks good to me. The only sticking point is we are about to merge Generic CMake build support, which should make sharing CMake code between IDF and non-IDF CMake projects much easier. However, those changes will conflict with this PR.

This PR is still useful though, because there may be situations where CMake libraries or other projects want to test at CMake runtime whether they are running inside IDF or inside a generic CMake project.

Once we merge the Generic CMake support (next few days), I'll cherry-pick this PR onto our master branch and resolve the conflicts, and we can merge it that way.

@PerMalmberg
Copy link
Contributor Author

Now that the new CMake changes have been merged, could these changes please be cherry picked into master, @projectgus ?

@projectgus
Copy link
Contributor

@PerMalmberg Yes, sorry for the hold-up. We got stuck discussing whether we should deprecate the ESP_PLATFORM name now that most variables are being namespaced as IDF_xxx. This will cherry-picked (either using this variable name or a more consistent one), soon.

@PerMalmberg
Copy link
Contributor Author

@projectgus Have you decided on the variable name yet?

@projectgus
Copy link
Contributor

After a long discussion, we're going to stick with ESP_PLATFORM for now. PR should be finally merged shortly... thanks for staying patient.

@PerMalmberg
Copy link
Contributor Author

After a long discussion, we're going to stick with ESP_PLATFORM for now. PR should be finally merged shortly... thanks for staying patient.

And thank you for the update. Looking forward to be able to compile for both ESP and Linux again.

igrr pushed a commit that referenced this pull request Jan 3, 2019
igrr pushed a commit that referenced this pull request Jan 3, 2019
(Handles case where idf.py is not being used.)

Ref #2601
igrr pushed a commit that referenced this pull request Jan 3, 2019
@PerMalmberg
Copy link
Contributor Author

@projectgus I'm closing this since your commits resolves this PR.

@projectgus
Copy link
Contributor

@PerMalmberg Thanks for that, and thanks for your patience while we worked through this change.

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

2 participants