search

andsens / bootstrap-vz

Bootstrap Debian images for virtualized environments
http://bootstrap-vz.readthedocs.io/
Other
263 stars 145 forks source link

Allow relative paths to workspace #465

Closed liori closed 6 years ago

liori commented 6 years ago

I like having my paths relative.

I've noticed that wherever a plugin wanted to allow relative paths, yet some tool used by that plugin required an absolute path, the path was converted from relative to absolute somewhere close to the place where the absolute path was required.

At first, I tried to do the same. I started patching all places that used the workspace path, and that would fail at runtime with just a relative path. But it turns out there's quite a lot of places that fail; my somewhat minimalistic test case already required 7 changes.

Then I realized it would be better to have the path relative to the manifest file, not just the current directory—just like the "vbox: Make guest additions path relative to manifest" patch. So, I'd essentially need to patch all the places.

So, I decided to just modify the manifest itself during the "parsing" step. I've noticed that the current code carefully avoids modifying the manifest data structure, probably for a good reason… but this just feels the right approach to me here. Though, maybe there is a better way?

andsens commented 6 years ago

Points for a creative and simple solution, but this is something that I would prefer to be fixed in a more thorough manner. See #444, maybe you can add your experience and get the PR going again? It introduces the concept of a workspace variable separate from the manifest, which represents the same thing but would work regardless of whether you specify the path in the manifest in a relative or absolute manner.