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

Fixed isUpdateAvailable in updateService.snap.ts to correctly identify snap version #64392

Merged
merged 2 commits into from Dec 20, 2018

Conversation

Projects
None yet
5 participants
@Kedstar99
Copy link

Kedstar99 commented Dec 5, 2018

Hello again,

I seem to have missed this line of code from #63716.

This change should occur for the exact same reason as the above PR.

In this case, the snap and product.applicationName is expecting the snap to be mounted at
/snap/code/current. It is instead mounted at /snap/vscode/current.

The non-null assertion operator should be safe as the environment variable is checked at construction. I Although a similar check also appears to occur in doQuitAndInstall. I have removed that check and used the non-null assertion operator.If this isn't correct, I am happy to change it.

Running "grep -nr '/snap/${product.applicationName}/current' . " does not produce anymore results.

I am sorry for missing this in the initial PR.

Krish De Souza
Snap update fixes in updateService.snap.ts
 - Fixed isUpdateAvailable in updateService.snap.ts to correctly identify snap version
 - Also fixed a small typo in doCheckForUpdates
 - Removed check in function doQuitAndInstall
@Kedstar99

This comment has been minimized.

Copy link
Author

Kedstar99 commented Dec 5, 2018

Hello again,

I was just looking at the shell script in snapUpdate.sh that is available in the code-insiders package.

At present it just seems to be doing the following.

#!/bin/sh
sleep 2
code-insiders

If instead it was written like the following, both the snapcrafters version and vscode-insiders should work and function identically. I have tested this works

#!/bin/sh
sleep 2
$SNAP_NAME

I hope this is useful.
Kind regards,
Krish

@joaomoreno

This comment has been minimized.

Copy link
Member

joaomoreno commented Dec 6, 2018

If instead it was written like the following, both the snapcrafters version and vscode-insiders should work and function identically. I have tested this works

Good to know, care to push that to the PR as well?

@msftclas

This comment has been minimized.

Copy link

msftclas commented Dec 6, 2018

CLA assistant check
All CLA requirements met.

@Kedstar99

This comment has been minimized.

Copy link
Author

Kedstar99 commented Dec 13, 2018

I have added the changes to snapUpdate.sh.

Is there anything else needed for this PR?

Without this patch, I am currently getting ENOENT: no such file or directory, lstat '/snap/code'.

@joaomoreno joaomoreno added this to the December/January 2019 milestone Dec 20, 2018

@joaomoreno joaomoreno merged commit d37f1c0 into Microsoft:master Dec 20, 2018

2 checks passed

VS Code #20181206.27 succeeded
Details
license/cla All CLA requirements met.
Details
@HankB

This comment has been minimized.

Copy link

HankB commented Dec 24, 2018

What is the implication for those experiencing the bug with 1.30.0? Does this break the update and if so, do we need to take action to work around? I'm thinking I just need to reinstall the snap package when an update is available.
Thanks!

@Kedstar99

This comment has been minimized.

Copy link
Author

Kedstar99 commented Dec 24, 2018

@HankB The snap package and update mechanism is external. The code that was broken was the function to check if a new version is available. If true, the code ATM is supposed to simply close and reopen the program.

You don't need to do anything but wait till the next version is released.

@doubledare704

This comment has been minimized.

Copy link

doubledare704 commented Jan 16, 2019

What is a plan of snap updates? It's a 16th of January, and that error is still annoying.

@Kedstar99

This comment has been minimized.

Copy link
Author

Kedstar99 commented Jan 16, 2019

@doubledare704 From what I can understand. It appears that this patch is lined up for the December/January 2019 milestone. I don't think this was included in the November Milestone but others can correct me otherwise.

At least that is my interpretation based on these milestone tags.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment