search

joltwallet / esp_littlefs

LittleFS port for ESP-IDF
MIT License
259 stars 97 forks source link

build: fix for the misuse of IDF_VER #126

Closed huming2207 closed 1 year ago

huming2207 commented 1 year ago

See: https://github.com/joltwallet/esp_littlefs/issues/125

BrianPugh commented 1 year ago

so somehow this only breaks esp-idf v4.2 (I just reran CI without this PR and it works fine).

That said, esp-idf v4.2 is at EOL in June 2023, and we're close enough to that that I'd be fine dropping support now.

That said, if the fix is trivial, we should just do the fix. Can you investigate a little bit?

If we deprecate v4.1 and v4.2, I can do that in another PR, then we can merge this PR.

Thankyou!

huming2207 commented 1 year ago

Hmm this is weird, I think it suppose to work as expected. Let me try logging the IDF_VER_NO_V and see what's the behaviour in ESP-IDF 4.2.

Meanwhile I don't have the environment set up for 4.2 locally so I will amend the commit in this PR and let the CI does the test for me.

huming2207 commented 1 year ago

I think I fixed it. The original cmake file's version detection logic is wrong: https://github.com/joltwallet/esp_littlefs/blob/5a375263fb07219d2a3748fc43067803754c7c67/project_include.cmake#L52

Change the VERSION_GREATER to VERSION_GREATER_EQUAL fixes the issue.

huming2207 commented 1 year ago

See this CI result: https://github.com/huming2207/esp_littlefs/actions/runs/5063107970/jobs/9089346490

huming2207 commented 1 year ago

Anyway, problem resolved, please review again. Thanks!

See here for my latest CI result: https://github.com/huming2207/esp_littlefs/actions/runs/5063135805

BrianPugh commented 1 year ago

excellent! thank you so much! will be merging and releasing momentarily.

BrianPugh commented 1 year ago

included in v1.5.5