Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extension workspaceStorage folder is missing metaData file #41408

Closed
dbaeumer opened this issue Jan 10, 2018 · 16 comments
Closed

Extension workspaceStorage folder is missing metaData file #41408

dbaeumer opened this issue Jan 10, 2018 · 16 comments
Assignees
Labels
api bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Milestone

Comments

@dbaeumer
Copy link
Member

dbaeumer commented Jan 10, 2018

Each folder had in the past a meta data file which now seems to be missing.

@bpasero any idea/reason why that got removed. Did it get moved somewhere else?

Not having that file anymore makes it very hard to clean up even with user interaction the workspaceFolder storage.

@bpasero
Copy link
Member

bpasero commented Jan 10, 2018

@dbaeumer it would help if you could point me to the place where this metadata file is created. I assume it would be close to ExtensionStoragePath?

@bpasero bpasero added the info-needed Issue requires more information from poster label Jan 10, 2018
@dbaeumer
Copy link
Member Author

@bpasero pointed me to that commit: cabd8ad#diff-c61682f360a18cf690ac59fe3f625db7

@jrieken any reason why you removed creating the file ?

@bpasero bpasero removed their assignment Jan 12, 2018
@jrieken
Copy link
Member

jrieken commented Jan 12, 2018

@jrieken any reason why you removed creating the file ?

Sorry, must have been an oversight when doing the sync to async fix. What is the file being used for and what should it contain? I can then bring it back.

@dbaeumer
Copy link
Member Author

The file contained the path to the workspace or workspace file plus the workspace uud. Format was JSON with { workspacePath: string; uuid: string }

@jrieken jrieken added bug Issue identified by VS Code Team member as probable bug api and removed info-needed Issue requires more information from poster labels Jan 15, 2018
@jrieken jrieken added this to the January 2018 milestone Jan 15, 2018
jrieken added a commit that referenced this issue Jan 15, 2018
@jrieken
Copy link
Member

jrieken commented Jan 15, 2018

@dbaeumer I have pushed c677bf3 to restore the meta.jon-file creation. Still, we don't have a workspace-path anymore (because it can be many now). Also, with https://github.com//Microsoft/vscode/commit/73dad2ebb9ee82fd40d358fbdf39e1403ced342e @bpasero made the folder be the workspace id. So, with my changes there is no new information as the meta.json basically repeats the name of it's containing folder. Is there anything else you want to have? What will the data be used for?

@dbaeumer
Copy link
Member Author

@jrieken the purpose of the information is to be able to clean old workspace storage information with user interaction and to let users know which storage folders take how much space (both not implemented). So we need to either now the workspace folder or the workspace meta data file location (the .code) file. To my knowledge each multi root workspace gets its own workspace storage location. If a folder in 'mounted' into two multi root workspaces there workspace storage is not shared. @bpasero this is correct?

@jrieken
Copy link
Member

jrieken commented Jan 16, 2018

So we need to either now the workspace folder or the workspace meta data file location (the .code) file.

Yeah, but you can move that file around and I am not sure what happens then to the workspace... @bpasero will the id stay stable or not?

jrieken added a commit that referenced this issue Jan 17, 2018
@jrieken
Copy link
Member

jrieken commented Jan 17, 2018

@dbaeumer I have pushed a change to also store the workspace name, tho not the path as that information is not available. So, it will look like this

{
  "id": "7b10e36039a1c1cbd389829dfe50ec17",
  "name": "fofof (Workspace)"
}

We could also add the folders of that workspace but that information might change over time...

@jrieken jrieken closed this as completed Jan 17, 2018
@bpasero
Copy link
Member

bpasero commented Jan 17, 2018

@dbaeumer @jrieken each workspace gets its own workspace ID that is derived from the location on disk (single folder: path of folder, workspace: path of workspace config file).

Moving the workspace config file = new ID.

@dbaeumer
Copy link
Member Author

@jrieken @bpasero not having the location on disk will it make very hard for a user to decide if the workspace storage is still needed. We can't even check if the workspace is still available since we don't know where to look. So to implement a user acceptable workspace clean story we would need the location on disk. @bpasero do you store that anywhere and I could then use the ID to get the revers mapping?

@bpasero
Copy link
Member

bpasero commented Jan 18, 2018

@dbaeumer we have a list of recently opened folders and workspaces, but that one is bounded to 100, so it may not include all entries. And this list can get stale when the workspace was moved.

I suggest to just store the location of the workspace file as meta data and then this could be presented to the user to ask if the storage is still needed. There is a configuration property on the workspace that has the location.

@dbaeumer
Copy link
Member Author

That makes sense to me. So in case of a folder workspace we store the folder location and in case of a workspace with n folder we store the location of the .code file.

@jrieken can you add this

@jrieken
Copy link
Member

jrieken commented Jan 23, 2018

check

@jrieken jrieken assigned bpasero, sandy081 and jrieken and unassigned bpasero and sandy081 Jan 23, 2018
@jrieken
Copy link
Member

jrieken commented Jan 23, 2018

I have tried to expose the configuration property on IWorkspace which seems to be a little harder....

@bpasero
Copy link
Member

bpasero commented Jan 23, 2018

@jrieken isn't this just about making configuration optional in IWorkspaceData? Since a workspace can also be a folder without configuration, I think it is fine to do so.

@jrieken jrieken closed this as completed Jan 24, 2018
jrieken added a commit that referenced this issue Jan 24, 2018
jrieken added a commit that referenced this issue Jan 24, 2018
@dbaeumer
Copy link
Member Author

dbaeumer commented Feb 1, 2018

The file now contains

{
  "id": "9a06e95ec93b72725824346f17c608a7",
  "configuration": null,
  "name": "41408"
}

which is IMO not enough information to find the folder on disk. Do I miss something?

@bpasero bpasero added the verified Verification succeeded label Feb 2, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Mar 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

4 participants