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

Write the .last_revision stamp under $SNAP_USER_DATA. #66

Merged
merged 1 commit into from
Jun 29, 2017

Conversation

oSoMoN
Copy link
Member

@oSoMoN oSoMoN commented Jun 28, 2017

This ensures the desktop launchers can also be used with classic snaps.

This ensures the desktop launchers can also be used with classic snaps.
timsueberkrueb added a commit to lirios/platform-snap that referenced this pull request Jun 28, 2017
@didrocks
Copy link
Member

I am a little bit vary about that one. Are you sure that the desktop launchers are working properly in a classic environment? I see multiple things, like relocatable home and so on which are incompatible with classic snaps where home is the real user's home.

Why do you need the desktop launchers for classic? If it's just few libraries you embeeded, you shouldn't use any desktop-* parts, but just export LD_LIBRARY_PATH and such, wdyt?

@oSoMoN
Copy link
Member Author

oSoMoN commented Jun 29, 2017

Well I started using the desktop helpers seeing that I had lots of duplicated code in my custom launcher, not just LD_LIBRARY_PATH exports. It might not be a great idea as you point out though, and in fact this prompts me to reconsider the use of classic confinement at all.

In any case, that change should be harmless for strictly confined snaps, and if the desktop launchers are not meant to be used by classic snap I think this should be clearly documented, to avoid skewed expectations.

@didrocks
Copy link
Member

You're right. Also, that was the only place I was using ~ instead of $SNAP_USER_DATA. So, in any case, that's a good change to have, independant of classic confinement or not.

Thanks!

@didrocks didrocks merged commit 05d8142 into ubuntu:master Jun 29, 2017
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