Allow project relative workspace paths #191

Open Cynical-Optimist opened 4 years ago

See original issue on GitLab In GitLab by [Gitlab user @tristanvb] on Jan 15, 2018, 07:04

While I find this to be a very odd practice that I would not recommend, it seems that people like to open workspace directories as subdirectories of the BuildStream project itself.

The one benefit I can see to this is that if the workspace is a project relative subdirectory; then one could move the project to a new directory and carry the workspace along with it as a subdirectory.

But for this to work without breaking the workspace/project relationship, BuildStream would have to support serializing and interpreting the workspace directory as a project relative path.

To make this work:

bst workspace open must check if the specified path is a subdirectory of the project
- If this is the case, then we can avoid calling os.path.abspath() here
When interpreting a workspace directory loaded from the project state directory (project/.bst/...), we should be initially using os.path.join(project_directory, workspace_directory); this can be done without consideration of whether the path was initially relative or absolute, due to the behavior of os.path.join()

Update by [Gitlab user @toscalix] : a proposal that covers this case has been sent to the mailing list.

In GitLab by [Gitlab user @knownexus] on Jan 22, 2018, 18:04

mentioned in merge request !251

In GitLab by [Gitlab user @cs-shadow] on Feb 1, 2018, 18:29

mentioned in merge request !154

In GitLab by [Gitlab user @LaurenceUrhegyi] on Jun 1, 2018, 14:55

marked this issue as related to #222

In GitLab by [Gitlab user @LaurenceUrhegyi] on Jun 1, 2018, 14:55

marked this issue as related to #229

In GitLab by [Gitlab user @LaurenceUrhegyi] on Jun 1, 2018, 14:56

marked this issue as related to #222

In GitLab by [Gitlab user @LaurenceUrhegyi] on Jun 1, 2018, 14:56

removed the relation with #222

In GitLab by [Gitlab user @LaurenceUrhegyi] on Jun 1, 2018, 14:56

marked this issue as related to #229

In GitLab by [Gitlab user @LaurenceUrhegyi] on Jun 1, 2018, 14:56

removed the relation with #229

In GitLab by [Gitlab user @knownexus] on Jun 18, 2018, 14:58

mentioned in merge request !504

In GitLab by [Gitlab user @LaurenceUrhegyi] on Jul 13, 2018, 09:51

assigned to [Gitlab user @knownexus]

In GitLab by [Gitlab user @knownexus] on Aug 6, 2018, 16:18

mentioned in commit d5646acf453968959fceb98e79f465a193d64cc3

In GitLab by [Gitlab user @phildawson] on Aug 7, 2018, 11:48

mentioned in commit c06745e877e87992fdc197b759a8e3382b202188

In GitLab by [Gitlab user @phildawson] on Aug 8, 2018, 09:55

mentioned in commit 5a7a6404c3139441a48242ed01d05d882920dcd7

In GitLab by [Gitlab user @knownexus] on Aug 9, 2018, 19:15

mentioned in commit 70a82dcf17289a47acd26d9c8da320c29e6b2b58

In GitLab by [Gitlab user @knownexus] on Aug 10, 2018, 10:33

mentioned in commit 78bce629143fc7a013de7f7c72078d6805db95cb

In GitLab by [Gitlab user @knownexus] on Aug 10, 2018, 11:18

mentioned in commit db0478abc83afb96dc1c146668ad1e8eb7fa671f

In GitLab by [Gitlab user @knownexus] on Aug 10, 2018, 12:12

closed via merge request !504

In GitLab by [Gitlab user @knownexus] on Aug 10, 2018, 12:12

mentioned in commit 04cee9a96df64c5e74d62db2bfbf28d7f248ae60

In GitLab by [Gitlab user @LaurenceUrhegyi] on Aug 10, 2018, 16:53

Re-opening as I'd like use to use the 'Verify' label here and close this ticket once someone has verified the behaviour in master.

In GitLab by [Gitlab user @LaurenceUrhegyi] on Aug 10, 2018, 16:53

reopened

In GitLab by [Gitlab user @tristanvb] on Aug 13, 2018, 10:53

[Gitlab user @LaurenceUrhegyi] thanks for pinging on this one.

I may be mistaken, but by a brief reading, it seems that the patch in !504:

Assumes that if the user creates a workspace as a subdirectory of the project, that the user wants it to be recorded as a project relative path
Does not allow at all using a relative path such as ../workspaces/foo.bst/

Unless I'm mistaken, I think this misses the mark.

I've added https://gitlab.com/BuildStream/buildstream/merge_requests/504/diffs#note_94123621 and https://gitlab.com/BuildStream/buildstream/merge_requests/504/diffs#note_94121771 to the closed merge request for additional context.

In GitLab by [Gitlab user @tristanvb] on Aug 13, 2018, 10:58

Additional comment after re-reading the issue description.

I think this issue itself outlines the behavior of !504 accurately, but is inopportunely titled.

This issue only calls for support for opening workspaces as subdirectories of the project. The patch achieves this correctly, but I don't think it's really good enough.

I would never recommend that anyone ever open a workspace inside the project directory. The project directory is the BuildStream input data; the workspace does not belong inside of it.

In GitLab by [Gitlab user @knownexus] on Aug 14, 2018, 16:34

[Gitlab user @tristanvb] as i mentioned in https://gitlab.com/BuildStream/buildstream/merge_requests/504#note_94454909 I disagree that we shouldn't have workspaces inside the project directory, i think that should be the only place they should exist.

However that is a different topic. I suggest that a new issue should be created for having external relative paths so that discussion can happen there, instead of holding this ticket open or changing it. (My opinion is that relative external workspaces is a bad idea/won't work well)

In GitLab by [Gitlab user @LaurenceUrhegyi] on Aug 14, 2018, 16:59

[Gitlab user @knownexus] this conversation should be parked until a design change is proposed for workspaces:

https://mail.gnome.org/archives/buildstream-list/2018-August/msg00023.html

https://mail.gnome.org/archives/buildstream-list/2018-August/msg00033.html

In GitLab by [Gitlab user @toscalix] on Aug 28, 2018, 11:50

The proposal has been sent to the mailing list for discussion.

In GitLab by [Gitlab user @toscalix] on Aug 28, 2018, 11:51

changed the description

In GitLab by [Gitlab user @knownexus] on Oct 2, 2018, 17:01

unassigned [Gitlab user @knownexus]

In GitLab by [Gitlab user @tristanvb] on Aug 4, 2019, 18:27

mentioned in commit f011ed315aefd4da1da5f4ba9862f7d59fa77927

In GitLab by [Gitlab user @tristanvb] on Aug 4, 2019, 18:28

mentioned in commit eefcd7a2c8158d907db634f9963120d9b533b2a7

In GitLab by [Gitlab user @tristanvb] on Aug 4, 2019, 18:29

mentioned in commit 2b571c6a4fb420c487d5302d47065311355d0991

In GitLab by [Gitlab user @tristanvb] on Aug 4, 2019, 19:11

mentioned in commit 0a457a720999ad6f3621020ef31f7fff9389fd5d

buildstream-migration / bst-staging

Allow project relative workspace paths #191