You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by Metuchen <gi...@git.apache.org> on 2017/07/13 15:06:04 UTC
[GitHub] incubator-guacamole-server pull request #105: GUACAMOLE-344: switching to up...
GitHub user Metuchen opened a pull request:
https://github.com/apache/incubator-guacamole-server/pull/105
GUACAMOLE-344: switching to upstream libvncserver because it properly enables VeNCrypt
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Metuchen/incubator-guacamole-server master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-guacamole-server/pull/105.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #105
----
commit 332492f9c8b0dea0ada9327c410cc0ce46c7c46a
Author: Mark Tomich <mt...@netsilo.metuchen.dyndns.org>
Date: 2017-07-13T14:59:58Z
GUACAMOLE-344: switching to upstream libvncserver because it properly enables VeNCrypt
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-server pull request #105: GUACAMOLE-344: switching to up...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-server/pull/105#discussion_r131229659
--- Diff: src/guacd-docker/bin/build-libvncserver.sh ---
@@ -0,0 +1,36 @@
+#!/bin/sh -e
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+##
+## @fn build-libvncserver.sh
+##
+## Builds the source of libvncserver
+##
+## @param BUILD_DIR
+## The working directory to download, extract and build the libvncserver distribution
--- End diff --
Same here - missing period. Please also wrap your lines at 80 columns where possible (and where doing so doesn't hurt readability). It's sometimes a tough call whether to wrap a line of code, but there are almost no cases where doing so would be bad for the documentation.
See the style of: https://github.com/apache/incubator-guacamole-server/blob/6236eb8f98ff96c495ae94cb4fe3279c202db465/src/guacd-docker/bin/build-guacd.sh#L27-L29
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-server pull request #105: GUACAMOLE-344: switching to up...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-server/pull/105#discussion_r131227442
--- Diff: src/guacd-docker/bin/build-libvncserver.sh ---
@@ -0,0 +1,36 @@
+#!/bin/sh -e
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+##
+## @fn build-libvncserver.sh
+##
+## Builds the source of libvncserver
+##
+## @param BUILD_DIR
+## The working directory to download, extract and build the libvncserver distribution
+##
+
+BUILD_DIR="$1"
+
+cd "$BUILD_DIR"
+curl -L https://github.com/LibVNC/libvncserver/archive/LibVNCServer-0.9.11.tar.gz | tar -zx
+cd libvncserver-LibVNCServer-0.9.11
+./autogen.sh
+make install
--- End diff --
Missing `ldconfig` after `make install`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-server pull request #105: GUACAMOLE-344: switching to up...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-server/pull/105#discussion_r127611740
--- Diff: Dockerfile ---
@@ -77,6 +76,12 @@ COPY . "$BUILD_DIR"
# Build guacamole-server from local source
RUN yum -y install $BUILD_DEPENDENCIES && \
+ curl -L https://codeload.github.com/LibVNC/libvncserver/tar.gz/LibVNCServer-0.9.11 | tar -zx && \
--- End diff --
Why `codeload.github.com`? Is this URL documented anywhere?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-server pull request #105: GUACAMOLE-344: switching to up...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-server/pull/105#discussion_r131227877
--- Diff: src/guacd-docker/bin/build-libvncserver.sh ---
@@ -0,0 +1,36 @@
+#!/bin/sh -e
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+##
+## @fn build-libvncserver.sh
+##
+## Builds the source of libvncserver
--- End diff --
A bit of a nitpick, but to match the style of the rest of our source, there needs to be a period at the end of the sentence here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-server pull request #105: GUACAMOLE-344: switching to up...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-server/pull/105#discussion_r127611769
--- Diff: Dockerfile ---
@@ -77,6 +76,12 @@ COPY . "$BUILD_DIR"
# Build guacamole-server from local source
RUN yum -y install $BUILD_DEPENDENCIES && \
+ curl -L https://codeload.github.com/LibVNC/libvncserver/tar.gz/LibVNCServer-0.9.11 | tar -zx && \
+ cd libvncserver-LibVNCServer-0.9.11 && \
+ ./autogen.sh && \
+ make install && \
+ cd .. && \
+ rm -fr libvncserver-LibVNCServer-0.9.11 && \
--- End diff --
Rather than embedding the build steps within the `Dockerfile`, they should instead be within a documented shell script. Besides being cleaner and easier to maintain, this removes the need to `cd ..` to restore the working directory.
See: https://github.com/apache/incubator-guacamole-server/blob/6236eb8f98ff96c495ae94cb4fe3279c202db465/src/guacd-docker/bin/build-guacd.sh
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-server pull request #105: GUACAMOLE-344: switching to up...
Posted by Metuchen <gi...@git.apache.org>.
Github user Metuchen commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-server/pull/105#discussion_r127764352
--- Diff: Dockerfile ---
@@ -77,6 +76,12 @@ COPY . "$BUILD_DIR"
# Build guacamole-server from local source
RUN yum -y install $BUILD_DEPENDENCIES && \
+ curl -L https://codeload.github.com/LibVNC/libvncserver/tar.gz/LibVNCServer-0.9.11 | tar -zx && \
--- End diff --
I got that link by browsing github... but I think this URL might be more appropriate: https://github.com/LibVNC/libvncserver/archive/LibVNCServer-0.9.11.tar.gz
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---