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.

Sign up for GitHub

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

Jump to bottom

Roaming profile still lives; isn't deleted by the sync service #3291

Closed
DHowett-MSFT opened this issue Oct 22, 2019 · 1 comment · Fixed by #5199
Closed

Roaming profile still lives; isn't deleted by the sync service #3291

DHowett-MSFT opened this issue Oct 22, 2019 · 1 comment · Fixed by #5199
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-0 Bugs that we consider release-blocking/recall-class (P0) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-DataLoss A bug that causes the user's data to be lost, unintentionally
Milestone
Terminal v1.0

Comments

@DHowett-MSFT
Copy link
Contributor

When WT deletes the profile from the roaming directory, that deletion never gets pushed up to the settings sync host. That's probably by design. Unfortunately, it means that on new installations, releases-old profiles can be magically resurrected from the ether.

We could (perhaps should) write a tombstone file to the roaming settings folder indicating that we've moved the settings out of it and not to try again.
@DHowett-MSFT DHowett-MSFT added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. Severity-DataLoss A bug that causes the user's data to be lost, unintentionally labels Oct 22, 2019
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Oct 22, 2019
@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Oct 22, 2019
@zadjii-msft zadjii-msft added this to the Terminal v1.0 milestone Dec 9, 2019
@zadjii-msft zadjii-msft added the Priority-1 A description (P1) label Dec 9, 2019
@cinnamon-msft cinnamon-msft added v1-Scrubbed Priority-0 Bugs that we consider release-blocking/recall-class (P0) and removed Priority-1 A description (P1) labels Jan 23, 2020
DHowett-MSFT pushed a commit that referenced this issue Jan 30, 2020
@DHowett
Settings: Stop resurrecting dead roaming profiles
46aae81 
Fixes #3291.
Refs #1069.
DHowett-MSFT pushed a commit that referenced this issue Mar 30, 2020
@DHowett
Settings: Stop resurrecting dead roaming profiles
1be9feb 
Fixes #3291.
Refs #1069.
DHowett-MSFT pushed a commit that referenced this issue Mar 31, 2020
@DHowett
Settings: Stop resurrecting dead roaming profiles
961793f 
Fixes #3291.
Refs #1069.
@DHowett-MSFT DHowett-MSFT mentioned this issue Mar 31, 2020
Merged
@ghost ghost added the In-PR This issue has a related PR label Mar 31, 2020
@ghost ghost closed this as completed in #5199 Apr 1, 2020
ghost pushed a commit that referenced this issue Apr 1, 2020
@DHowett
rename profiles.json to settings.json, clean up the defaults (#5199)
64489b1 
This pull request migrates `profiles.json` to `settings.json` and removes the legacy roaming AppData settings migrator.

It also:

* separates the key bindings in defaults.json into logical groups
* syncs the universal terminal defaults with the primary defaults
* removes some stray newlines that ended up at the beginning of settings.json and defaults.json

Fixes #5186.
Fixes #3291.

### categorize key bindings

### sync universal with main

### kill stray newlines in template files

### move profiles.json to settings.json

This commit also changes Get*Settings from returning a string to
returning a std::filesystem::path. We gain in expressiveness without a
loss in clarity (since path still supports .c_str()).

NOTE: I tried to do an atomic rename with the handle open, but it didn't
work for reparse points (it moves the destination of a symbolic link
out into the settings folder directly.)

(snip for atomic rename code)

```c++
auto path{ pathToSettingsFile.wstring() };
auto renameBufferSize{ sizeof(FILE_RENAME_INFO) + (path.size() * sizeof(wchar_t)) };
auto renameBuffer{ std::make_unique<std::byte[]>(renameBufferSize) };
auto renameInfo{ reinterpret_cast<FILE_RENAME_INFO*>(renameBuffer.get()) };
renameInfo->Flags = FILE_RENAME_FLAG_REPLACE_IF_EXISTS | FILE_RENAME_FLAG_POSIX_SEMANTICS;
renameInfo->RootDirectory = nullptr;
renameInfo->FileNameLength = gsl::narrow_cast<DWORD>(path.size());
std::copy(path.cbegin(), path.cend(), std::begin(renameInfo->FileName));

THROW_IF_WIN32_BOOL_FALSE(SetFileInformationByHandle(hLegacyFile.get(),
                          FileRenameInfo,
                          renameBuffer.get(),
                          gsl::narrow_cast<DWORD>(renameBufferSize)));
```

(end snip)

### Stop resurrecting dead roaming profiles
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Apr 1, 2020
@ghost
Copy link

ghost commented Apr 22, 2020

🎉This issue was addressed in #5199, which has now been successfully released as Windows Terminal Preview v0.11.1121.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-0 Bugs that we consider release-blocking/recall-class (P0) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-DataLoss A bug that causes the user's data to be lost, unintentionally
Projects
None yet
Milestone
Terminal v1.0
Development

Successfully merging a pull request may close this issue.

rename profiles.json to settings.json, clean up the defaults microsoft/terminal
3 participants
@DHowett-MSFT @zadjii-msft @cinnamon-msft