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

Make sure args.getSettingsPath() returns the correct directory #3956

Merged
merged 1 commit into from
Jun 6, 2021

Conversation

daschuer
Copy link
Member

@daschuer daschuer commented Jun 6, 2021

This makes sure that getSettingsPath() always return the right directory.
This is done by writing it back using args.getSettingsPathSet() when it was changed by sandboxing.
I have also moved the code into main, to make sure that CmdlineArgs::Instance()->getSettingsPath() can be used unconditionally.

@daschuer
Copy link
Member Author

daschuer commented Jun 6, 2021

Not sure if this is a 2.3 candidate, because I am not able to test it. The fixed bug is obvious though.

@daschuer daschuer changed the title Make sure getSettingsPath() returns the correct directory Make sure args.getSettingsPath() returns the correct directory Jun 6, 2021
@daschuer daschuer force-pushed the migrateOldSettings branch from a53a65b to 5b1cbe8 Compare June 6, 2021 10:44
Copy link
Contributor

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me. Other opinions?

@uklotzde uklotzde added this to the 2.3.0 milestone Jun 6, 2021
@uklotzde
Copy link
Contributor

uklotzde commented Jun 6, 2021

Ok, then let's merge and hope for the best ;) LGTM

@uklotzde uklotzde merged commit a5bc82a into mixxxdj:2.3 Jun 6, 2021
@uklotzde
Copy link
Contributor

uklotzde commented Jun 7, 2021

The changes need to be merged to main now. Non-trivial merge conflicts.

@daschuer
Copy link
Member Author

daschuer commented Jun 7, 2021

Yes, it looks like the call to Sandbox::checkSandboxed() has been lost in main. I think that is wrong, right?
I will restore it.

@uklotzde
Copy link
Contributor

uklotzde commented Jun 7, 2021

Thanks!

daschuer added a commit to daschuer/mixxx that referenced this pull request Jun 19, 2021
… introduced by mixxxdj#3956

This is a bandaid. The reintroduced issue of settings path access before checking the sandbox will be fixed in a follow up PR
daschuer added a commit to daschuer/mixxx that referenced this pull request Jun 19, 2021
… introduced by mixxxdj#3956

This is a band aid. The reintroduced issue of settings path access before checking the sandbox will be fixed in a follow up PR.
daschuer added a commit to daschuer/mixxx that referenced this pull request Jun 19, 2021
… introduced by mixxxdj#3956

This is a band aid. The reintroduced issue of settings path access before checking the sandbox will be fixed in a follow up PR.
daschuer added a commit to daschuer/mixxx that referenced this pull request Jun 19, 2021
… introduced by mixxxdj#3956

This is a band aid. The reintroduced issue of settings path access before checking the sandbox will be fixed in a follow up PR.
@daschuer daschuer deleted the migrateOldSettings branch August 1, 2021 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants