Skip to content
This repository was archived by the owner on Apr 1, 2022. It is now read-only.

libraries/compile-examples: install dependencies from local path using symlinks #81

Merged
merged 1 commit into from
Sep 11, 2020
Merged

libraries/compile-examples: install dependencies from local path using symlinks #81

merged 1 commit into from
Sep 11, 2020

Conversation

per1234
Copy link
Contributor

@per1234 per1234 commented Sep 10, 2020

Previously, the shared function used to install dependencies from local paths used copy instead of symlinks.

The switch to using copy for everything was done for a couple of reasons:

  • Some functions were already using copy/move to install because they worked from temporary folders that only had a lifetime within the function call
  • Use of symlinks on Windows requires the script to be run as administrator, which makes it less friendly to contributors or users running the script locally who are using Windows

However, the previous symlinks were essential for the deltas feature to work for libraries or platforms under test because these are installed to a different location than the repository that is checked out to the delta base and head refs.

When the project under test is a sketch, symlinks are not necessary, since the sketch is run in place, which is why this bug was not detected via minimal the informal integration testing done on the change.

…g symlinks

Previously, the shared function used to install dependencies from local paths used copy instead of symlinks.

The switch to using copy for everything was done for a couple of reasons:

- Some functions were already using copy/move to install because they worked from temporary folders that are deleted by the context manager
- Use of symlinks on Windows requires the script to be run as administrator, which makes it less friendly to contributors or users running the script locally who are using Windows

However, the previous symlinks were essential for the deltas feature to work for libraries or platforms under test because these are installed to a different location than the repository that is checked out to the delta and head refs.

When the project under test is a sketch, symlinks are not necessary, since the sketch is run in place, which is why this bug was not detected via minimal informal integration tests.

The first reason to copy listed above is easily overcome by changing the code to create temporary folders with the required lifespan.

The second remains, but it's easy enough to find the solution to the error message resulting from running the script without the necessary permissions.
@per1234 per1234 added the bug label Sep 10, 2020
@codecov-commenter
Copy link

Codecov Report

Merging #81 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##            master       #81    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files            2         2            
  Lines          626      1545   +919     
==========================================
+ Hits           626      1545   +919     
Impacted Files Coverage Δ
...ompile-examples/compilesketches/compilesketches.py 100.00% <100.00%> (ø)
...ples/compilesketches/tests/test_compilesketches.py 100.00% <100.00%> (ø)
...t-size-deltas/reportsizedeltas/reportsizedeltas.py
...as/reportsizedeltas/tests/test_reportsizedeltas.py

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 56d149b...30dc93a. Read the comment docs.

Copy link
Contributor

@aentinger aentinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 Thank you @per1234 🚀

@aentinger aentinger merged commit fc23253 into arduino:master Sep 11, 2020
@per1234 per1234 deleted the install-via-symlink branch September 11, 2020 05:48
@rsora rsora added the type: imperfection Perceived defect in any part of project label Sep 22, 2021
@per1234 per1234 added the topic: code Related to content of the project itself label Sep 29, 2021
@per1234 per1234 self-assigned this Nov 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants