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

Desktop: Upgrade to precompiled Qt 5.12.9 with OpenSSL 1.1.1g #231

Merged
merged 3 commits into from
Jul 22, 2020

Conversation

misch7
Copy link
Member

@misch7 misch7 commented Jul 22, 2020

Upgrade to precompiled Qt 5.12.9

  • Also implement env vars for specifying the Qt and OpenSSL versions, to get rid of some duplication as they are used very often.

  • Add command line argument to enable building Qt from source with having to modify the Dockerfile:

    Run docker build with --build-arg BUILD_QT=1 to build Qt from source (default: not set) on a dedicated build machine:

    docker build -t client-5.12 . --build-arg BUILD_QT=1

    Then for example run the following command to get the Qt tarball:

    docker run \
      --name build-qt-tmp \
      --rm \
      -v $(pwd):/output \
      client-5.12 \
      /bin/bash -c 'cp /qt-bin* /output/'
    

    Puts the tarball into the current working directory.

Other tweaks:

@@ -2,6 +2,15 @@ FROM ubuntu:xenial

MAINTAINER Roeland Jago Douma <roeland@famdouma.nl>

# Run 'docker build' with '--build-arg BUILD_QT=1' to build Qt from source (default: not set)
Copy link
Member

Choose a reason for hiding this comment

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

I'm not asking to change this in that PR, I'm just trying to solve what's still a mystery to me. :-)

Why are we not building it in the image always? Why are we building it once and downloading this separately? I'd expect that build to be a one time pain anyway, it's not the case?

Copy link
Member Author

Choose a reason for hiding this comment

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

See here:

https://github.com/nextcloud/docker-ci/blob/f7700ce/client/Dockerfile-5.12#L115-L122

;-)

Docker Hub has a forced timeout for jobs, they usually terminate after around 240 minutes. So I had to come up with a solution to get us an image anyway ;)

Copy link
Member

Choose a reason for hiding this comment

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

Ah annoying indeed... And what about making our own base image that we upload? or Docker hub forces us to let it build itself?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yepp ;/
Might be possible to do it this way, that just would take more time to tinker with again ;)

client/Dockerfile-5.12 Outdated Show resolved Hide resolved
- Upgrade to precompiled Qt 5.12.9

- Also implement env vars for specifying the Qt and OpenSSL versions, to get rid
  of some duplication as they are used very often.

- Add command line argument to enable building Qt from source with having to modify
  the Dockerfile:

  Run 'docker build' with '--build-arg BUILD_QT=1' to build Qt from source (default: not set)
  on a dedicated build machine:

    docker build -t client-5.12 . --build-arg BUILD_QT=1

  Then for example run the following command to get the Qt tarball:

    docker run \
      --name build-qt-tmp \
      --rm \
      -v $(pwd):/output \
      client-5.12 \
      /bin/bash -c 'cp /qt-bin* /output/'

  Puts the tarball into the current working directory.

Signed-off-by: Michael Schuster <michael@schuster.ms>
Signed-off-by: Michael Schuster <michael@schuster.ms>
See: nextcloud/desktop#2213

Signed-off-by: Michael Schuster <michael@schuster.ms>
@misch7 misch7 force-pushed the client_qt-5.12.9-precompiled branch from 8a343ec to d052154 Compare July 22, 2020 12:09
@misch7 misch7 requested a review from er-vin July 22, 2020 12:16
misch7 added a commit to nextcloud/desktop that referenced this pull request Jul 22, 2020
Also remove the jq package installation from the AppImage build script because
the package is included in the new Docker image (see nextcloud/docker-ci#231).

Signed-off-by: Michael Schuster <michael@schuster.ms>
@@ -150,3 +150,5 @@ RUN if [ "$BUILD_QT" != "1" ] ; then echo Download precompiled Qt. && \
tar -xvf ${QT_TARBALL} && \
rm ${QT_TARBALL} \
; fi

Copy link
Member

Choose a reason for hiding this comment

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

I guess you didn't want to keep that line and the next? (they appear alone in the jq commit)

Copy link
Member Author

Choose a reason for hiding this comment

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

Intended, wanted to have a clear separator for whatever will come after the Qt-related stuff ;)

Copy link
Member

Choose a reason for hiding this comment

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

Ah OK, fair enough.

@misch7 misch7 requested a review from er-vin July 22, 2020 13:21
@misch7 misch7 merged commit fb4c6b7 into master Jul 22, 2020
@misch7 misch7 deleted the client_qt-5.12.9-precompiled branch July 22, 2020 14:24
PraMiD pushed a commit to PraMiD/nextcloud-desktop that referenced this pull request Dec 30, 2020
Also remove the jq package installation from the AppImage build script because
the package is included in the new Docker image (see nextcloud/docker-ci#231).

Signed-off-by: Michael Schuster <michael@schuster.ms>
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