Place url-scm download files in .bob-download

[rhubert]

Separate the compressed input files from the extracted files in the workspace in preparation of bundling the sources.

[codecov]

Codecov Report

Attention: Patch coverage is 92.59259% with 8 lines in your changes missing coverage. Please review.

Project coverage is 88.86%. Comparing base (2d829b4) to head (c619d27).

Files with missing lines	Patch %	Lines
pym/bob/scm/url.py	92.30%	8 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #606   +/-   ##
=======================================
  Coverage   88.86%   88.86%           
=======================================
  Files          48       48           
  Lines       15474    15551   +77     
=======================================
+ Hits        13751    13820   +69     
- Misses       1723     1731    +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jkloetzke]

Uhh, changing the stable-variant-ids test is a hinting that it breaks backwards compatibility. Moving the downloaded file into a subdirectory could break existing recipes. Even if the file is extracted, the recipe can still rely on the file being present today. So at least we must have a policy to guard this.

I would propose to have the downloaded file not in a magic hidden directory but instead completely outside of workspace. We could have a downloads next to workspace that stores the downloaded file and the canary. That would make it completely transparent. No need to exclude the directory from hashing and the workspace appears clean...

[rhubert]

I would propose to have the downloaded file not in a magic hidden directory but instead completely outside of workspace.

I also thought about this. The main reason for placing it inside was the attic logic for workspaces. I'm unsure how to thread the download-folder files if the recipe changes. Simply delete it? Or do we need some attic logic there as well?

[jkloetzke]

I would propose to have the downloaded file not in a magic hidden directory but instead completely outside of workspace.

I also thought about this. The main reason for placing it inside was the attic logic for workspaces. I'm unsure how to thread the download-folder files if the recipe changes.

Good point. Indeed, this will require some extra logic to keep the downloads directory from collecting garbage. But this will probably enable further optimizations in the future. If the downloaded file is already available locally (e.g. when using a preMirror), we can directly extract such archives in the future without even copying the file in the workspace.

Simply delete it? Or do we need some attic logic there as well?

I think we can simply delete it. The "downloads" directory probably needs to have the same structure as the "workspace". This is required anyway because there might be multiple url SCMs in the same workspace. I guess adding a postAttic method to SCMs which is called in the attic code path can simply delete the downloaded file.

[jkloetzke]

👍 Just some minor nits...

[jkloetzke]

Is the os.path.abspath really necessary? If true, then I would appreciate a comment why. It's somewhat unexpected.

[rhubert]

the reason for abspath was, that os.path.exists has some trouble if the path contains ..:

>>> import os
>>> os.path.exists('foo/bar/../..')
False

however I think using os.path.normpath instead of abspath is the cleaner solution?

[jkloetzke]

I think the trouble comes from the fact that it fails if some of the path elements does not exist. Like for example if the user removed workspace from dev/[...]/1/workspace/../download then the path really does not exist. So I think you're right, os.path.normpath is more appropriate. Still, I would appreciate a comment because it's not obvious. 😉

[rhubert]

This sounds like a good explanation :) I looked into the documentation but couldn't find anything for os.path.exists. But I found a note for os.makedirs saying it's also confused by ... So I added normalize there as well. And a comment ;)