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

Allow read-write access to configuration and download directories when installed as a Snap #972

Closed
wants to merge 2 commits into from

Conversation

richardneish
Copy link

We need write access to the $HOME/.config/exercism/ directory for exercism config and $HOME/exercism is the default download directory so we need write access there as well.

This should resolve issue #945

We need write access to the $HOME/.config/exercism/ directory for `exercism config` and $HOME/exercism is the default download directory so we need write access there as well.

This should resolve issue #945
.goreleaser.yml Outdated
personal-files:
write:
- $HOME/.config/exercism/
- $HOME/exercism/
Copy link
Member

Choose a reason for hiding this comment

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

This is only the default installation directory, and should not be hard coded, i think, but should gather that from the configuration file. Otherwise we are likely going to have intermittent permission problems if the user changes the workspace.

Copy link
Author

Choose a reason for hiding this comment

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

We can allow write access to any subdirectory of $HOME - would that be better?

Copy link
Member

Choose a reason for hiding this comment

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

I suspect that is better. Perhaps the configuration file is not yet written at this point in time, to be able to get the workspace from the configuration.

Base automatically changed from master to main January 28, 2021 22:32
Copy link
Contributor

@ekingery ekingery left a comment

Choose a reason for hiding this comment

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

It is possible that the ideal solution is for a tighter restriction than $HOME. However, given the installation troubles we've seen, I think it is worthwhile to merge this as-is.

@ekingery
Copy link
Contributor

ekingery commented Feb 9, 2021

Thanks for the contribution @richardneish! It should be reflected in the next release of the CLI, which should correspond with the Exercism V3 launch. The build should pass after getting merged with main.

@ekingery
Copy link
Contributor

ekingery commented Feb 9, 2021

In order to merge, the build needs to pass. In order for the build to pass, we need to merge the latest changes from main. There was no base branch to merge into, so I created a separate PR for getting this merged: #980

@ekingery ekingery closed this Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants