Skip to content
This repository has been archived by the owner on Jun 10, 2019. It is now read-only.

Allow relative paths to workspace #465

Closed
wants to merge 1 commit into from

Conversation

liori
Copy link
Contributor

@liori liori commented Mar 10, 2018

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
Copy link
Owner

andsens commented May 2, 2018

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.

@andsens andsens closed this May 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants