BrianPugh / belay

Belay is a python library that enables the rapid development of projects that interact with hardware via a micropython-compatible board.
Apache License 2.0
240 stars 13 forks source link

Fix bug in device.sync that prevented it from working on some drives #141

Closed TimAidley closed 1 year ago

TimAidley commented 1 year ago

device.sync wasn't working when the source folder was on a different drive than the temp directory.

I think the intention of the code was to get the path without the drive specifier by getting the relative path of it when compared to the drive specifier. However the temp file driver specifier was being passed through rather than the source file one.

BrianPugh commented 1 year ago

great find! thanks for the fix; will merge and cut a release as soon as CI does its thing.

BrianPugh commented 1 year ago

I can see what you were intending; let me fix this and add a unit test

BrianPugh commented 1 year ago

so I know you meant to do src_file.anchor, but does this solve the bug that came up? What did an example src_file path look like that caused an issue?

TimAidley commented 1 year ago

Traceback (most recent call last):
  File "D:\src\timsimconnect2\main.py", line 68, in <module>
    libraries_setup()
  File "D:\src\timsimconnect2\main.py", line 11, in libraries_setup
    device.sync("adafruit_seesaw")
  File "D:\src\timsimconnect2\venv\lib\site-packages\belay\device.py", line 412, in sync
    for (src_file, src_hash), dst_file, dst_hash in zip(src_files_and_hashes, dst_files, dst_hashes):
  File "C:\Users\Tim\AppData\Local\Programs\Python\Python310\lib\concurrent\futures\_base.py", line 609, in result_iterator
    yield fs.pop().result()
  File "C:\Users\Tim\AppData\Local\Programs\Python\Python310\lib\concurrent\futures\_base.py", line 439, in result
    return self.__get_result()
  File "C:\Users\Tim\AppData\Local\Programs\Python\Python310\lib\concurrent\futures\_base.py", line 391, in __get_result
    raise self._exception
  File "C:\Users\Tim\AppData\Local\Programs\Python\Python310\lib\concurrent\futures\thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
  File "D:\src\timsimconnect2\venv\lib\site-packages\belay\device.py", line 399, in _preprocess_src_file_hash_helper
    return preprocess_src_file_hash(tmp_dir, src_file, minify, mpy_cross_binary)
  File "D:\src\timsimconnect2\venv\lib\site-packages\belay\device_sync_support.py", line 101, in preprocess_src_file_hash
    src_file = preprocess_src_file(*args, **kwargs)
  File "D:\src\timsimconnect2\venv\lib\site-packages\belay\device_sync_support.py", line 84, in preprocess_src_file
    transformed = tmp_dir / src_file.relative_to(tmp_dir.anchor) if src_file.is_absolute() else tmp_dir / src_file
  File "C:\Users\Tim\AppData\Local\Programs\Python\Python310\lib\pathlib.py", line 818, in relative_to
    raise ValueError("{!r} is not in the subpath of {!r}"
ValueError: 'D:\\src\\timsimconnect2\\adafruit_seesaw\\__init__.py' is not in the subpath of 'C:\\' OR one path is relative and the other is absolute.```
BrianPugh commented 1 year ago

gotcha, thanks! fix coming right up.

BrianPugh commented 1 year ago

pushed fix, can you confirm that this fixes the issue you are running into? If so, i'll merge and cut a new release.

codecov-commenter commented 1 year ago

Codecov Report

Merging #141 (40ec9d5) into main (239bbf1) will not change coverage. The diff coverage is 100.00%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@           Coverage Diff           @@
##             main     #141   +/-   ##
=======================================
  Coverage   70.54%   70.54%           
=======================================
  Files          38       38           
  Lines        1691     1691           
  Branches      374      374           
=======================================
  Hits         1193     1193           
+ Misses        425      423    -2     
- Partials       73       75    +2     
Flag Coverage Δ
unittests 70.49% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
belay/device_sync_support.py 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

TimAidley commented 1 year ago

whoops I realize I changed it to src_dir rather than src_file in the PR, but I see you spotted the error!

Anyway, looks like your change is working here.

BrianPugh commented 1 year ago

This is now released with v0.21.1! Thanks again!