Skip to content

Auto-disable (deep) caching of core builds if needed #479

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Nov 11, 2019

When build-path and cache-path are on different partitions it will not be possible (on windows) to reference them with a relative path. This commit allows this by automatically disabling the aggressive core caching.

Previously an error was emitted and the build interrupted.

Should fix #464

When build-path and cache-path are on different partitions it will not
be possible (on windows) to reference them with a relative path.
This commit allows this by automatically disabling the aggressive core
caching.

Previously an error was emitted and the build interrupted.
@cmaglie cmaglie self-assigned this Nov 11, 2019
@matthijskooijman
Copy link
Collaborator

Do we need to refer to them with a relative path? I guess absolute paths add command length, so that is not ideal, but maybe that is ok for just the core? Or are there other problems with absolute paths?

@facchinm
Copy link
Member

@matthijskooijman It was just about commandline length, so using the absolute path should solve the issue if coreBuildCachePath.RelTo(ctx.BuildPath) fails

@facchinm facchinm merged commit d3c1d50 into arduino:master Nov 11, 2019
@cmaglie
Copy link
Member Author

cmaglie commented Nov 11, 2019

The problem is that platform.txt uses (example taken from samd core):

 recipe.c.combine.pattern="{compiler.path}{compiler.c.elf.cmd}"  "-L{build.path}" ...
   -o "{build.path}/{build.project_name}.elf" {object_files} ...
   -lm "{build.path}/{archive_file}" -Wl,--end-group

so we need "{build.path}/{archive_file}" with archive_file relative to build.path

BTW on other parts of platform.txt there is also:

# archive_file_path is needed for backwards compatibility with IDE 1.6.5 or older, IDE 1.6.6 or newer overrides this value
archive_file_path={build.path}/{archive_file}
recipe.ar.pattern="{compiler.path}{compiler.ar.cmd}" {compiler.ar.flags} {compiler.ar.extra_flags} "{archive_file_path}" "{object_file}"

so in theory we should only use archive_file_path instead of {build.path}/{archive_file}, but this is not true even for our cores.
So a full fix should check for {archive_file} usage in combine recipe and do the right action consequently.

Anyway this is a lot more work for a very edge case.

@cmaglie cmaglie deleted the auto-disable-core-caching-if-needed branch November 11, 2019 14:10
@cmaglie cmaglie added this to the 0.7.0 milestone Nov 11, 2019
cmaglie added a commit to arduino/arduino-builder that referenced this pull request Nov 11, 2019
- Auto-disable (deep) caching of core builds if needed
  arduino/arduino-cli#479

- Print "multiple libraries found for header.h" message only when it really happens.
  arduino/arduino-cli#478
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Windows] Sketch compilation and core cache cannot live in different partitions
3 participants