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

if esp-idf is a submodule, then everything except this check works #438

Closed

Conversation

edmund-huber
Copy link
Contributor

Instead of forking this repo, and then customizing on top of it, (and then later having to reconcile changes), I have a root repo with all my goodies and then an esp-idf submodule.

It just works! Except that esp-idf's make process does a check for ${IDF_PATH}/.git -- which doesn't exist when esp-idf is a submodule. I don't really see the point of this check, since if esp-idf cannot be manipulated by git, any number of git commands in the makefile will fail and the build will stop.

@CLAassistant
Copy link

CLAassistant commented Mar 17, 2017

CLA assistant check
All committers have signed the CLA.

@projectgus
Copy link
Contributor

projectgus commented Mar 20, 2017

Hi Edmund,

Thanks for raising this! The way you're using esp-idf is great, and we definitely want to support this configuration.

The reason for the explicit check and error is that this may be the first error message new users get from IDF (for example, if they click "Download" on github then they get a non-functional zip file as github downloads don't include any submodules). So the intention was to make it more helpful than a git error, which would then needs to be interpeted by the new user (possibly someone who hasn't used git before).

Changing the -d to -e (from 'is directory' to 'exists') should fix the problem while keeping the more specific message, as submodules have a .git file rather than a .git directory. What do you think?

Angus

@edmund-huber
Copy link
Contributor Author

You're right, I misspoke in that .git does exist if you're in a submodule -- only that it's not a directory. What you're saying sounds good to me.

Side node: git didn't start making gitfiles for submodules until 1.7.8 apparently: https://github.com/git/git/blob/master/Documentation/RelNotes/1.7.8.txt#L109 . Which was 2013.

@projectgus
Copy link
Contributor

LGTM, have cherry-picked into our review & merge queue.

@projectgus projectgus added the Status: Pending blocked by some other factor label Mar 22, 2017
igrr pushed a commit that referenced this pull request Mar 23, 2017
@projectgus
Copy link
Contributor

Cherry-picked in 5272bd8, thanks!

@projectgus projectgus closed this Mar 24, 2017
@igrr igrr removed the Status: Pending blocked by some other factor label Apr 12, 2017
0xFEEDC0DE64 pushed a commit to 0xFEEDC0DE64/esp-idf that referenced this pull request May 5, 2021
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.

4 participants