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

common/desktop-exports: don't dereference target symlink on upgrades #7

Merged

Conversation

jdstrand
Copy link
Contributor

On install, $SNAP_USER_DATA/.themes is created and points to
$SNAP/usr/share/themes. On upgrade $SNAP_USER_DATA is copied from the previous
dir (eg, 'x1') to the new version (eg, 'x2') such that $SNAP_USER_DATA/.themes
points to the previous version's $SNAP/usr/share/themes. There is logic to
account for this in common/desktop-exports and it tries to:

ln -sf $SNAP/usr/share/themes $SNAP_USER_DATA/.themes

Because $SNAP_USER_DATA/.themes is a valid symlink, it is dereferenced and
tries to create $SNAP/usr/share/themes/themes, which is of course a readonly
directly and fails with:

ln: failed to create symbolic link '.../x2/.themes/themes': Read-only file
system

The script should instead use '-n' so that $SNAP_USER_DATA/.themes is updated
as intended.

On install, $SNAP_USER_DATA/.themes is created and points to
$SNAP/usr/share/themes. On upgrade $SNAP_USER_DATA is copied from the previous
dir (eg, 'x1') to the new version (eg, 'x2') such that $SNAP_USER_DATA/.themes
points to the previous version's $SNAP/usr/share/themes. There is logic to
account for this in common/desktop-exports and it tries to:

  ln -sf $SNAP/usr/share/themes $SNAP_USER_DATA/.themes

Because $SNAP_USER_DATA/.themes is a valid symlink, it is dereferenced and
tries to create $SNAP/usr/share/themes/themes, which is of course a readonly
directly and fails with:

  ln: failed to create symbolic link '.../x2/.themes/themes': Read-only file
  system

The script should instead use '-n' so that $SNAP_USER_DATA/.themes is updated
as intended.
@didrocks
Copy link
Member

didrocks commented Sep 7, 2016

That's indeed needed! Thanks a lot for the detailed description and patch!

@didrocks didrocks merged commit 30f0a5c into ubuntu:master Sep 7, 2016
@jdstrand
Copy link
Contributor Author

jdstrand commented Sep 7, 2016

Thanks!

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.

None yet

2 participants