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

fix update from pre-1.46 to post-1.46 not working in docker #1406

Merged
merged 1 commit into from
Jan 10, 2022
Merged

fix update from pre-1.46 to post-1.46 not working in docker #1406

merged 1 commit into from
Jan 10, 2022

Conversation

SoongJr
Copy link
Contributor

@SoongJr SoongJr commented Dec 26, 2021

The script previously used [[ ! -f /.docker-image ]] && { ... } as its last statement, which returns non-zero in docker (as it should), but this exit code is then used as the over-all exit code of the update script, thus failing the update whenever used in docker container. Fixed by adding the same exit 0 that's already there in all other update-scripts. Fixes issue #1405

Also removed a quote that I'm pretty sure shouldn't be there.

@SoongJr
Copy link
Contributor Author

SoongJr commented Dec 26, 2021

One minor concern I have is that the addition of set -e means that for non-docker images, if any of the following commands fails, the update will now be considered failed where it would previously ignore if any of the first two fails:

  systemctl daemon-reload
  systemctl enable refresh_notify_push.{path,service}
  systemctl restart refresh_notify_push.path

This is a change in behavior, but I hope a desirable one. Previously only the systemctl restart would have caused the update to abort as it's the last statement in the script and its return code becomes the code of the whole script.
Not sure the other two commands can even fail to be honest, just adding this for full disclosure. A test with a non-docker image would definitely be a good idea, but I don't have one ;)

@svenb1234
Copy link

@nachoparker @theCalcaholic could you please review this commit? Or is this not the correct fix for the issue?

Signed-off-by: SoongJr <SoongJr@googlemail.com>
@nachoparker nachoparker merged commit dd8fc3d into nextcloud:master Jan 10, 2022
@nachoparker
Copy link
Member

many thanks for this

@SoongJr SoongJr deleted the feature/fix_1.46.0 branch January 10, 2022 21:41
guandalf pushed a commit to guandalf/nextcloudpi that referenced this pull request Jan 12, 2022
…d#1406)

Signed-off-by: SoongJr <SoongJr@googlemail.com>
Signed-off-by: Stefano Guandalini <stefano.guandalini@proton.ch>
theCalcaholic pushed a commit that referenced this pull request Apr 8, 2022
Signed-off-by: SoongJr <SoongJr@googlemail.com>
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

3 participants