Fix linking failure for space in path in PlatformIO builder scripts #6464

maxgerhardt · 2022-03-22T11:10:43Z

By completing this PR sufficiently, you help us to improve the quality of Release Notes

Checklist

Please provide specific title of the PR describing the change, including the component name (eg. „Update of Documentation link on Readme.md“)
Please provide related links (eg. Issue, other Project, submodule PR..)
Please check Contributing guide

This entire section above can be deleted if all items are checked.

Summary

This PR modifies the linker flag that produces the .map file for the project. For projects which contain spaces, this previously resulted in an error (#6463).

Impact

Fixes compilation for projects that contain spaces in their path or enviornment name (in platformio.ini, [env:xxx]). The .map file will however now be the default firmware.map (or whatever PROGNAME is set to if modified by custom scripts).

The usage of basename() with environment subtition led to extremely weird bugs. This is fix here has been tested to work on both Windows on Linux. For the 'extreme case' of spaces in the project path and environment name, it correctly generates the flag on Windows

-Wl,-Map="C:\Users\Max\temp\Test Project With Spaces\.pio\build\esp32dev with spaces\firmware.map"

and on Linux

-Wl,-Map="/home/max/Test Project with Spaces/.pio/build/esp32dev with spaces/firmware.map"

Which would previously error out.

Related links

See #6463.

CLAassistant · 2022-03-22T11:10:48Z

All committers have signed the CLA.

maxgerhardt · 2022-03-22T11:13:33Z

P.S.: This is a hotfix intended to get it working / unblock people, a nicer fix would depend on platformio/platformio-core#4205.

Please wait a bit so that maybe a cleaner fix can be found there before merging.

mcspr · 2022-03-23T19:31:17Z

As mentioned in the platformio/platformio-core#4205 (comment)
Have you tried

    '-Wl,-Map',
    '-Wl,"${BUILD_DIR}/${PROGNAME}.map"'

in the LINKFLAGS?

maxgerhardt · 2022-03-23T19:32:07Z

This would likely be a cleaner and better fix, I will try in a moment.

maxgerhardt · 2022-03-23T19:45:52Z

Yes, this produces the correct flag

 -Wl,-Map -Wl,"C:\Users\Max\temp\Test Project With Spaces\.pio\build\esp32dev/firmware.map"

and the file is created at the proper place.

Still a difference from the what the original script would do with Test Project With Spaces.map as filename, but I think this is way better..

maxgerhardt · 2022-03-23T20:04:43Z

Wow no, this actually does not work. On Linux where the path seperator is /, it produces

-Wl,-Map "-Wl, "/home/max/Test" Project with Spaces/.pio/build/esp32dev with spaces/firmware.map"

there is that dead dreaded " in the middle of the path again and the whole quoting of the argument. This is more complicated than I thought.

maxgerhardt · 2022-03-23T20:06:41Z

The code '-Wl,-Map="%s"' % join("${BUILD_DIR}", "${PROGNAME}.map") works however.. I'll leave it at that.

mcspr · 2022-03-23T20:14:18Z

Strange, I was testing with the exact configuration where there were spaces in both project and env name :/

-Wl,-Map -Wl,"/home/max/dev/espurna test dir/code/.pio/build/esp8266 1m git base/firmware.map"

was created just fine in the wsl env

maxgerhardt · 2022-03-23T20:14:55Z

And that was with pio upgrade --dev latest core? (+ pio platform update latest all other packages)?

PS my reference platformio.ini is

[env:esp32dev with spaces]
platform = https://github.com/Jason2866/platform-espressif32.git
board = esp32dev
framework = arduino

mcspr · 2022-03-23T20:30:14Z

Just made sure that I fetched all of the above and did a pio upgrade --dev
Plus, the change I mentioned for the $framework/tools/platformio-build-esp32.py

-Wl,-Map -Wl,"/home/max/dev/esp32 test dir/.pio/build/esp32dev with spaces/firmware.map"

🤷

Jason2866 · 2022-03-23T20:32:34Z

I will do a build with this changes and update

platform = https://github.com/Jason2866/platform-espressif32.git

So more people can test

EDIT: repo is updated

maxgerhardt · 2022-03-23T20:35:00Z

This is too magical. Now I changed the line back to what @mcspr suggeted, did a pio run -t clean && pio run and now the code works too and generates

 -Wl,-Map -Wl,"/home/max/Test Project with Spaces/.pio/build/esp32dev with spaces/firmware.map"

Wow... I have really no idea now.

Edit: Works on Windows too.

But in any case, the current version of the PR with the two arguments combined also works equivalently.. 🤷

Jason2866 · 2022-03-23T22:01:00Z

@maxgerhardt The fix works fine. 👍 Our map move and naming Platformio script is still working 🤞

github-actions · 2022-03-28T10:48:28Z

Unit Test Results

0 files 0 suites 0s ⏱️
0 tests 0 ✔️ 0 💤 0 ❌

Results for commit c686b66.

♻️ This comment has been updated with latest results.

me-no-dev · 2022-03-28T10:52:52Z

@maxgerhardt can you please confirm that all is good?

Jason2866 · 2022-03-28T12:25:19Z

@maxgerhardt @me-no-dev I have included this PR (with S3) 5 days ago and we have no negative feedback from users when compiling Tasmota with this change.
Tested with spaces in path. The error happend before is fixed.

Jason2866

Tested and verified

me-no-dev · 2022-03-28T12:48:23Z

Thanks guys :) Merged!

esp-dl: master d949350 esp-dsp: master 07aa7b1 esp-rainmaker: master 5af4f64 esp-sr: master d05cf97 esp32-camera: master 86a4951 esp_littlefs: master 5f0d614

Jason2866 · 2022-03-28T16:10:26Z

@me-no-dev Latest commit 31510f4 did revert this PR

me-no-dev · 2022-03-28T16:20:27Z

the lib-builder must have grabbed arduino before merge. will reapply

Jason2866 · 2022-03-28T16:54:59Z

@me-no-dev No it is here https://github.com/espressif/esp32-arduino-lib-builder/blob/master/tools/copy-libs.sh#L310

Jason2866 · 2022-03-28T16:57:55Z

Mhh, i have to look if this PR ever was in a build.

me-no-dev · 2022-03-28T18:12:47Z

@me-no-dev No it is here https://github.com/espressif/esp32-arduino-lib-builder/blob/master/tools/copy-libs.sh#L310

yup that too

in lib builder

maxgerhardt added 3 commits March 22, 2022 11:55

Hotfix linking failure for paths containing spaces

a147e44

Hotfix linking failure for paths containing spaces (esp32c3)

ef9997a

Hotfix linking failure for paths containing spaces (ESP32S2)

06084b4

maxgerhardt changed the title ~~Hotfix linking failure for space in path~~ Hotfix linking failure for space in path in PlatformIO builder scripts Mar 22, 2022

maxgerhardt added 3 commits March 23, 2022 21:08

Use alternative path and escaping method

77bfefd

Use alternative path and escaping method

e57febe

Use alternative path and escaping method

f585452

maxgerhardt changed the title ~~Hotfix linking failure for space in path in PlatformIO builder scripts~~ Fix linking failure for space in path in PlatformIO builder scripts Mar 23, 2022

Merge branch 'master' into patch-1

68e84ea

me-no-dev added 2 commits March 28, 2022 13:48

Update platformio-build-esp32c3.py

bad1c4e

add esp32s3 changes

c686b66

Jason2866 reviewed Mar 28, 2022

View reviewed changes

me-no-dev merged commit 51040cc into espressif:master Mar 28, 2022

Jason2866 referenced this pull request Mar 28, 2022

IDF release/v4.4 c29343eb94 (#6493)

31510f4

esp-dl: master d949350 esp-dsp: master 07aa7b1 esp-rainmaker: master 5af4f64 esp-sr: master d05cf97 esp32-camera: master 86a4951 esp_littlefs: master 5f0d614

Jason2866 added a commit to Jason2866/esp32-arduino-lib-builder that referenced this pull request Mar 29, 2022

Integrate PR espressif/arduino-esp32#6464

287e674

in lib builder

me-no-dev pushed a commit to espressif/esp32-arduino-lib-builder that referenced this pull request Mar 29, 2022

Integrate PR espressif/arduino-esp32#6464 (#66)

d83e0cc

in lib builder

me-no-dev added a commit that referenced this pull request Mar 29, 2022

Re-apply #6464

5c5a112

maxgerhardt mentioned this pull request Mar 30, 2022

Linking project with spaces in path fails #6463

Closed

1 task

maxgerhardt mentioned this pull request Apr 10, 2022

Escaping linker arguments with quotes impossible to do correctly platformio/platformio-core#4205

Closed

1 task

valeros mentioned this pull request Apr 18, 2022

Cannot build with feature/arduino-upstream platformio/platform-espressif32#690

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix linking failure for space in path in PlatformIO builder scripts #6464

Fix linking failure for space in path in PlatformIO builder scripts #6464

maxgerhardt commented Mar 22, 2022 •

edited

Loading

CLAassistant commented Mar 22, 2022 •

edited

Loading

maxgerhardt commented Mar 22, 2022

mcspr commented Mar 23, 2022

maxgerhardt commented Mar 23, 2022

maxgerhardt commented Mar 23, 2022

maxgerhardt commented Mar 23, 2022

maxgerhardt commented Mar 23, 2022

mcspr commented Mar 23, 2022

maxgerhardt commented Mar 23, 2022 •

edited

Loading

mcspr commented Mar 23, 2022

Jason2866 commented Mar 23, 2022 •

edited

Loading

maxgerhardt commented Mar 23, 2022 •

edited

Loading

Jason2866 commented Mar 23, 2022

github-actions bot commented Mar 28, 2022 •

edited

Loading

me-no-dev commented Mar 28, 2022

Jason2866 commented Mar 28, 2022

Jason2866 left a comment

me-no-dev commented Mar 28, 2022

Jason2866 commented Mar 28, 2022

me-no-dev commented Mar 28, 2022

Jason2866 commented Mar 28, 2022

Jason2866 commented Mar 28, 2022

me-no-dev commented Mar 28, 2022

Fix linking failure for space in path in PlatformIO builder scripts #6464

Fix linking failure for space in path in PlatformIO builder scripts #6464

Conversation

maxgerhardt commented Mar 22, 2022 • edited Loading

Checklist

Summary

Impact

Related links

CLAassistant commented Mar 22, 2022 • edited Loading

maxgerhardt commented Mar 22, 2022

mcspr commented Mar 23, 2022

maxgerhardt commented Mar 23, 2022

maxgerhardt commented Mar 23, 2022

maxgerhardt commented Mar 23, 2022

maxgerhardt commented Mar 23, 2022

mcspr commented Mar 23, 2022

maxgerhardt commented Mar 23, 2022 • edited Loading

mcspr commented Mar 23, 2022

Jason2866 commented Mar 23, 2022 • edited Loading

maxgerhardt commented Mar 23, 2022 • edited Loading

Jason2866 commented Mar 23, 2022

github-actions bot commented Mar 28, 2022 • edited Loading

Unit Test Results

me-no-dev commented Mar 28, 2022

Jason2866 commented Mar 28, 2022

Jason2866 left a comment

Choose a reason for hiding this comment

me-no-dev commented Mar 28, 2022

Jason2866 commented Mar 28, 2022

me-no-dev commented Mar 28, 2022

Jason2866 commented Mar 28, 2022

Jason2866 commented Mar 28, 2022

me-no-dev commented Mar 28, 2022

maxgerhardt commented Mar 22, 2022 •

edited

Loading

CLAassistant commented Mar 22, 2022 •

edited

Loading

maxgerhardt commented Mar 23, 2022 •

edited

Loading

Jason2866 commented Mar 23, 2022 •

edited

Loading

maxgerhardt commented Mar 23, 2022 •

edited

Loading

github-actions bot commented Mar 28, 2022 •

edited

Loading