You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datasketches.apache.org by GitBox <gi...@apache.org> on 2020/07/16 15:43:18 UTC
[GitHub] [incubator-datasketches-postgresql] phstudy opened a new pull request #28: add dockerfile
phstudy opened a new pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28
add dockerfile
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on pull request #28: add Dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#issuecomment-675233090
> The base image (debian buster) only supports LLVM 7.0 which failed to build extension for postgresql 11+.
Fixed. Adding the following code in Makefile:
```
COMPILE.cxx.bc = $(CLANG) -xc++ -Wno-ignored-attributes $(BITCODE_CXXFLAGS) $(CPPFLAGS) -emit-llvm -c
%.bc : %.cpp
$(COMPILE.cxx.bc) -o $@ $<
$(LLVM_BINPATH)/opt -module-summary -f $@ -o $@
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on a change in pull request #28: add dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r455992247
##########
File path: Dockerfile
##########
@@ -0,0 +1,58 @@
+# 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.
+
+FROM postgres:9.6
+
+MAINTAINER dev@datasketches.apache.org
+
+ENV PG_MAJOR 9.6
+
+ADD . /datasketches-postgresql
+WORKDIR /datasketches-postgresql
+
+RUN echo "===> Adding prerequisites..." && \
+ apt-get update -y && \
+ DEBIAN_FRONTEND=noninteractive \
+ apt-get install --no-install-recommends --allow-downgrades -y -q \
+ ca-certificates \
+ build-essential git \
+ postgresql-server-dev-9.6 \
+ libpq-dev=$PG_MAJOR\* libpq5=$PG_MAJOR\* libecpg-dev && \
+ echo "===> Building datasketches..." && \
+ git clone https://github.com/apache/incubator-datasketches-cpp.git datasketches-cpp && \
Review comment:
fixed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] AlexanderSaydakov commented on a change in pull request #28: add dockerfile
Posted by GitBox <gi...@apache.org>.
AlexanderSaydakov commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r456647690
##########
File path: Dockerfile
##########
@@ -0,0 +1,72 @@
+# 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.
+
+ARG PG_MAJOR_VERSION=9.6
+ARG PG_MINOR_VERSION=9.6
Review comment:
how are these used? why we need them separately? isn't it like 9 - major, 6 - minor?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on a change in pull request #28: add Dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r471991493
##########
File path: Dockerfile
##########
@@ -0,0 +1,72 @@
+# 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.
+
+ARG PG_MAJOR_VERSION=9.6
+ARG PG_MINOR_VERSION=9.6
Review comment:
I have made some update:
1. Default base image version was changed from `9.6` to `latest`.
2. `PG_MAJOR_VERSION` & `PG_MINOR_VERSION` variables were removed and I parse them from pre-installed packages.
3. Only `BASE_IMAGE_VERSION` variable is remained and it is overridable, example:
```
docker build --pull --build-arg BASE_IMAGE_VERSION=10.14 -t datasketch-postgres:10.14 .
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] leerho commented on a change in pull request #28: add dockerfile
Posted by GitBox <gi...@apache.org>.
leerho commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r456750952
##########
File path: Dockerfile
##########
@@ -0,0 +1,72 @@
+# 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.
+
+ARG PG_MAJOR_VERSION=9.6
+ARG PG_MINOR_VERSION=9.6.18
+ARG BASE_IMAGE_VERSION=9.6.18
+
+ARG DATASKETCHES_CPP_VERSION=2.0.0-incubating
+
+FROM postgres:$BASE_IMAGE_VERSION
+
+MAINTAINER dev@datasketches.apache.org
+
+ARG PG_MAJOR_VERSION
+ARG PG_MINOR_VERSION
+ARG DATASKETCHES_CPP_VERSION
+
+ENV PG_MAJOR=$PG_MAJOR_VERSION
+ENV PG_MINOR=$PG_MINOR_VERSION
+ENV DATASKETCHES_CPP=$DATASKETCHES_CPP_VERSION
+
+
+ADD . /datasketches-postgresql
+WORKDIR /datasketches-postgresql
+
+RUN echo "===> Adding prerequisites..." && \
+ apt-get update -y && \
+ DEBIAN_FRONTEND=noninteractive \
+ apt-get install --no-install-recommends --allow-downgrades -y -q \
+ ca-certificates \
+ build-essential wget \
+ postgresql-server-dev-$PG_MAJOR=$PG_MINOR\* \
+ libpq-dev=$PG_MINOR\* libpq5=$PG_MINOR\* libecpg-dev && \
+ \
+ \
+ echo "===> Building datasketches..." && \
+ wget https://github.com/apache/incubator-datasketches-cpp/archive/$DATASKETCHES_CPP.tar.gz && \
Review comment:
Not necessarily. I would post this question on general@incubator.apache.org and see what the response is. I have been told by very senior folks at ASF that there are many older Apache projects that violate a lot of ASF policies but they have been grandfathered in. Also the policies themselves have changed over the years as ASF has grown. So the best advice would be to ask some of the senior folks like Justin Mclean, Dave Fischer, etc.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] AlexanderSaydakov merged pull request #28: add Dockerfile
Posted by GitBox <gi...@apache.org>.
AlexanderSaydakov merged pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] jmalkin commented on a change in pull request #28: add dockerfile
Posted by GitBox <gi...@apache.org>.
jmalkin commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r455924260
##########
File path: Dockerfile
##########
@@ -0,0 +1,58 @@
+# 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.
+
+FROM postgres:9.6
+
+MAINTAINER dev@datasketches.apache.org
+
+ENV PG_MAJOR 9.6
+
+ADD . /datasketches-postgresql
+WORKDIR /datasketches-postgresql
+
+RUN echo "===> Adding prerequisites..." && \
+ apt-get update -y && \
+ DEBIAN_FRONTEND=noninteractive \
+ apt-get install --no-install-recommends --allow-downgrades -y -q \
+ ca-certificates \
+ build-essential git \
+ postgresql-server-dev-9.6 \
+ libpq-dev=$PG_MAJOR\* libpq5=$PG_MAJOR\* libecpg-dev && \
+ echo "===> Building datasketches..." && \
+ git clone https://github.com/apache/incubator-datasketches-cpp.git datasketches-cpp && \
Review comment:
I'm pretty sure Apache policy means we'll need to pin this to an official release.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on a change in pull request #28: add Dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r471991493
##########
File path: Dockerfile
##########
@@ -0,0 +1,72 @@
+# 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.
+
+ARG PG_MAJOR_VERSION=9.6
+ARG PG_MINOR_VERSION=9.6
Review comment:
I have removed `PG_MAJOR_VERSION` & `PG_MINOR_VERSION` variables and I parse them from installed packages.
Only `BASE_IMAGE_VERSION` variable is remained and it is overridable.
build example:
`docker build --pull --build-arg BASE_IMAGE_VERSION=10.14 -t datasketch-postgres:10.14 .`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on a change in pull request #28: add dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r456691242
##########
File path: Dockerfile
##########
@@ -0,0 +1,72 @@
+# 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.
+
+ARG PG_MAJOR_VERSION=9.6
+ARG PG_MINOR_VERSION=9.6
Review comment:
`PG_MAJOR_VERSION` is part of package name, for example: `postgresql-server-dev-10` and `postgresql-server-dev-9.6`.
`PG_MINOR_VERSION` is specified version of package, for example: `postgresql-server-dev-10=10.13-1\*` and `postgresql-server-dev-10=9.6.18\*`.
`BASE_IMAGE_VERSION` is base image version.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy edited a comment on pull request #28: add Dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy edited a comment on pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#issuecomment-671856700
The base image (debian buster) only supports LLVM 7.0 which failed to build extension for postgresql 11+.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on a change in pull request #28: add Dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r471898214
##########
File path: Dockerfile
##########
@@ -0,0 +1,98 @@
+# 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.
+
+ARG PG_MAJOR_VERSION=12
+ARG PG_MINOR_VERSION=12.4
+ARG BASE_IMAGE_VERSION=12.4
Review comment:
The base image (debian buster) only supports LLVM 7.0 which failed to build extension for postgresql 11+.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy edited a comment on pull request #28: add Dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy edited a comment on pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#issuecomment-675233090
> The base image (debian buster) only supports LLVM 7.0 which failed to build extension for postgresql 11+.
Fixed. Adding the following code in Makefile:
```
COMPILE.cxx.bc = $(CLANG) -xc++ -Wno-ignored-attributes $(BITCODE_CXXFLAGS) $(CPPFLAGS) -emit-llvm -c
%.bc : %.cpp
$(COMPILE.cxx.bc) -o $@ $<
$(LLVM_BINPATH)/opt -module-summary -f $@ -o $@
```
This patch should also fix #25.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on a change in pull request #28: add Dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r471898459
##########
File path: Dockerfile
##########
@@ -0,0 +1,98 @@
+# 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.
+
+ARG PG_MAJOR_VERSION=12
+ARG PG_MINOR_VERSION=12.4
+ARG BASE_IMAGE_VERSION=12.4
Review comment:
Fixed. Adding the following code to Makefile:
```
COMPILE.cxx.bc = $(CLANG) -xc++ -Wno-ignored-attributes $(BITCODE_CXXFLAGS) $(CPPFLAGS) -emit-llvm -c
%.bc : %.cpp
$(COMPILE.cxx.bc) -o $@ $<
$(LLVM_BINPATH)/opt -module-summary -f $@ -o $@
```
This patch should also fix #25.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] leerho commented on a change in pull request #28: add dockerfile
Posted by GitBox <gi...@apache.org>.
leerho commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r456725858
##########
File path: Dockerfile
##########
@@ -0,0 +1,72 @@
+# 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.
+
+ARG PG_MAJOR_VERSION=9.6
+ARG PG_MINOR_VERSION=9.6.18
+ARG BASE_IMAGE_VERSION=9.6.18
+
+ARG DATASKETCHES_CPP_VERSION=2.0.0-incubating
+
+FROM postgres:$BASE_IMAGE_VERSION
+
+MAINTAINER dev@datasketches.apache.org
+
+ARG PG_MAJOR_VERSION
+ARG PG_MINOR_VERSION
+ARG DATASKETCHES_CPP_VERSION
+
+ENV PG_MAJOR=$PG_MAJOR_VERSION
+ENV PG_MINOR=$PG_MINOR_VERSION
+ENV DATASKETCHES_CPP=$DATASKETCHES_CPP_VERSION
+
+
+ADD . /datasketches-postgresql
+WORKDIR /datasketches-postgresql
+
+RUN echo "===> Adding prerequisites..." && \
+ apt-get update -y && \
+ DEBIAN_FRONTEND=noninteractive \
+ apt-get install --no-install-recommends --allow-downgrades -y -q \
+ ca-certificates \
+ build-essential wget \
+ postgresql-server-dev-$PG_MAJOR=$PG_MINOR\* \
+ libpq-dev=$PG_MINOR\* libpq5=$PG_MINOR\* libecpg-dev && \
+ \
+ \
+ echo "===> Building datasketches..." && \
+ wget https://github.com/apache/incubator-datasketches-cpp/archive/$DATASKETCHES_CPP.tar.gz && \
Review comment:
[https://infra.apache.org/release-download-pages](https://infra.apache.org/release-download-pages)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] leerho commented on a change in pull request #28: add dockerfile
Posted by GitBox <gi...@apache.org>.
leerho commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r456725744
##########
File path: Dockerfile
##########
@@ -0,0 +1,72 @@
+# 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.
+
+ARG PG_MAJOR_VERSION=9.6
+ARG PG_MINOR_VERSION=9.6.18
+ARG BASE_IMAGE_VERSION=9.6.18
+
+ARG DATASKETCHES_CPP_VERSION=2.0.0-incubating
+
+FROM postgres:$BASE_IMAGE_VERSION
+
+MAINTAINER dev@datasketches.apache.org
+
+ARG PG_MAJOR_VERSION
+ARG PG_MINOR_VERSION
+ARG DATASKETCHES_CPP_VERSION
+
+ENV PG_MAJOR=$PG_MAJOR_VERSION
+ENV PG_MINOR=$PG_MINOR_VERSION
+ENV DATASKETCHES_CPP=$DATASKETCHES_CPP_VERSION
+
+
+ADD . /datasketches-postgresql
+WORKDIR /datasketches-postgresql
+
+RUN echo "===> Adding prerequisites..." && \
+ apt-get update -y && \
+ DEBIAN_FRONTEND=noninteractive \
+ apt-get install --no-install-recommends --allow-downgrades -y -q \
+ ca-certificates \
+ build-essential wget \
+ postgresql-server-dev-$PG_MAJOR=$PG_MINOR\* \
+ libpq-dev=$PG_MINOR\* libpq5=$PG_MINOR\* libecpg-dev && \
+ \
+ \
+ echo "===> Building datasketches..." && \
+ wget https://github.com/apache/incubator-datasketches-cpp/archive/$DATASKETCHES_CPP.tar.gz && \
Review comment:
Downloading apache releases from github is not allowed. And to do it automatically requires a script to balance between mirrors. Please read [](https://infra.apache.org/release-download-pages).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] AlexanderSaydakov commented on pull request #28: add dockerfile
Posted by GitBox <gi...@apache.org>.
AlexanderSaydakov commented on pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#issuecomment-660280618
why PostgreSQL 9.6? can this be overridden easily?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on a change in pull request #28: add Dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r471991493
##########
File path: Dockerfile
##########
@@ -0,0 +1,72 @@
+# 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.
+
+ARG PG_MAJOR_VERSION=9.6
+ARG PG_MINOR_VERSION=9.6
Review comment:
I have made some update:
1. Default base image version was changed from `9.6` to `latest`.
2. `PG_MAJOR_VERSION` & `PG_MINOR_VERSION` arguments were removed and I parse them from pre-installed packages.
3. Only `BASE_IMAGE_VERSION` argument is remained and it is overridable, example:
```
docker build --pull --build-arg BASE_IMAGE_VERSION=10.14 -t datasketch-postgres:10.14 .
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on a change in pull request #28: add dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r456651085
##########
File path: Dockerfile
##########
@@ -0,0 +1,72 @@
+# 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.
+
+ARG PG_MAJOR_VERSION=9.6
+ARG PG_MINOR_VERSION=9.6
Review comment:
[PostgreSQL version policy](https://www.postgresql.org/support/versioning/) changed after 9.6, so 9.6 is also major version.
In order to match distribution version with docker base image, we have to separate the version.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on a change in pull request #28: add Dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r471991493
##########
File path: Dockerfile
##########
@@ -0,0 +1,72 @@
+# 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.
+
+ARG PG_MAJOR_VERSION=9.6
+ARG PG_MINOR_VERSION=9.6
Review comment:
I have made some update:
1. Default base image version was changed from `9.6` to `latest`.
2. `PG_MAJOR_VERSION` & `PG_MINOR_VERSION` arguments were removed and I parse them from pre-installed packages.
3. Only `BASE_IMAGE_VERSION` argument is remained and it is overridable, example:
```
docker build --pull --build-arg BASE_IMAGE_VERSION=10.14 -t datasketch-postgres:10.14 .
```
4. Add continuous integration with Docker on Travis CI
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy removed a comment on pull request #28: add Dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy removed a comment on pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#issuecomment-675233090
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy edited a comment on pull request #28: add Dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy edited a comment on pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#issuecomment-675233090
> The base image (debian buster) only supports LLVM 7.0 which failed to build extension for postgresql 11+.
Fixed. Adding the following code to Makefile:
```
COMPILE.cxx.bc = $(CLANG) -xc++ -Wno-ignored-attributes $(BITCODE_CXXFLAGS) $(CPPFLAGS) -emit-llvm -c
%.bc : %.cpp
$(COMPILE.cxx.bc) -o $@ $<
$(LLVM_BINPATH)/opt -module-summary -f $@ -o $@
```
This patch should also fix #25.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on a change in pull request #28: add Dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r456691242
##########
File path: Dockerfile
##########
@@ -0,0 +1,72 @@
+# 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.
+
+ARG PG_MAJOR_VERSION=9.6
+ARG PG_MINOR_VERSION=9.6
Review comment:
~~`PG_MAJOR_VERSION` is part of package name, for example: [`postgresql-server-dev-10`]https://packages.ubuntu.com/bionic/postgresql-server-dev-10) and `postgresql-server-dev-9.6`.
`PG_MINOR_VERSION` is specified version of package, for example: `postgresql-server-dev-10=10.13-1\*` and `postgresql-server-dev-10=9.6.18\*`.
`BASE_IMAGE_VERSION` is base image version.~~
##########
File path: Dockerfile
##########
@@ -0,0 +1,72 @@
+# 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.
+
+ARG PG_MAJOR_VERSION=9.6
+ARG PG_MINOR_VERSION=9.6
Review comment:
~~I have updated the default `PG_MINOR_VERSION` and `BASE_IMAGE_VERSION` to an accurate version. This should make it more easy to understand the purpose of those versions.~~
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on a change in pull request #28: add Dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r477020314
##########
File path: Makefile
##########
@@ -43,6 +43,13 @@ PG_CONFIG = pg_config
PGXS := $(shell $(PG_CONFIG) --pgxs)
include $(PGXS)
+# fix LLVM JIT compilation error
+COMPILE.cxx.bc = $(CLANG) -xc++ -Wno-ignored-attributes $(BITCODE_CXXFLAGS) $(CPPFLAGS) -emit-llvm -c
Review comment:
I have added llvm check before applying the patch.
Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on a change in pull request #28: add dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r456691242
##########
File path: Dockerfile
##########
@@ -0,0 +1,72 @@
+# 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.
+
+ARG PG_MAJOR_VERSION=9.6
+ARG PG_MINOR_VERSION=9.6
Review comment:
`PG_MAJOR_VERSION` is part of package, for example: `postgresql-server-dev-10` and `postgresql-server-dev-9.6`.
`PG_MINOR_VERSION` is specified version of package, for example: `postgresql-server-dev-10=10.13-1\*` and `postgresql-server-dev-10=9.6.18\*`.
`BASE_IMAGE_VERSION` is base image version.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on a change in pull request #28: add dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r456657120
##########
File path: Dockerfile
##########
@@ -0,0 +1,72 @@
+# 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.
+
+ARG PG_MAJOR_VERSION=9.6
+ARG PG_MINOR_VERSION=9.6
Review comment:
I have updated the default `PG_MINOR_VERSION` and `BASE_IMAGE_VERSION` to an accurate version. This should make it more easy to understand the purpose of those versions.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on a change in pull request #28: add dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r456736977
##########
File path: Dockerfile
##########
@@ -0,0 +1,72 @@
+# 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.
+
+ARG PG_MAJOR_VERSION=9.6
+ARG PG_MINOR_VERSION=9.6.18
+ARG BASE_IMAGE_VERSION=9.6.18
+
+ARG DATASKETCHES_CPP_VERSION=2.0.0-incubating
+
+FROM postgres:$BASE_IMAGE_VERSION
+
+MAINTAINER dev@datasketches.apache.org
+
+ARG PG_MAJOR_VERSION
+ARG PG_MINOR_VERSION
+ARG DATASKETCHES_CPP_VERSION
+
+ENV PG_MAJOR=$PG_MAJOR_VERSION
+ENV PG_MINOR=$PG_MINOR_VERSION
+ENV DATASKETCHES_CPP=$DATASKETCHES_CPP_VERSION
+
+
+ADD . /datasketches-postgresql
+WORKDIR /datasketches-postgresql
+
+RUN echo "===> Adding prerequisites..." && \
+ apt-get update -y && \
+ DEBIAN_FRONTEND=noninteractive \
+ apt-get install --no-install-recommends --allow-downgrades -y -q \
+ ca-certificates \
+ build-essential wget \
+ postgresql-server-dev-$PG_MAJOR=$PG_MINOR\* \
+ libpq-dev=$PG_MINOR\* libpq5=$PG_MINOR\* libecpg-dev && \
+ \
+ \
+ echo "===> Building datasketches..." && \
+ wget https://github.com/apache/incubator-datasketches-cpp/archive/$DATASKETCHES_CPP.tar.gz && \
Review comment:
I borrow the download code from [apache/fluo](https://github.com/apache/fluo-docker/blob/1ccfc40d652fc715dee33a563606014356cff595/Dockerfile#L40). It should fit the downloading policy.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on a change in pull request #28: add dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r456691242
##########
File path: Dockerfile
##########
@@ -0,0 +1,72 @@
+# 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.
+
+ARG PG_MAJOR_VERSION=9.6
+ARG PG_MINOR_VERSION=9.6
Review comment:
`PG_MAJOR_VERSION` is part of package name, for example: [`postgresql-server-dev-10`](https://packages.ubuntu.com/bionic/postgresql-server-dev-10) and `postgresql-server-dev-9.6`.
`PG_MINOR_VERSION` is specified version of package, for example: `postgresql-server-dev-10=10.13-1\*` and `postgresql-server-dev-10=9.6.18\*`.
`BASE_IMAGE_VERSION` is base image version.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on a change in pull request #28: add Dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r477020314
##########
File path: Makefile
##########
@@ -43,6 +43,13 @@ PG_CONFIG = pg_config
PGXS := $(shell $(PG_CONFIG) --pgxs)
include $(PGXS)
+# fix LLVM JIT compilation error
+COMPILE.cxx.bc = $(CLANG) -xc++ -Wno-ignored-attributes $(BITCODE_CXXFLAGS) $(CPPFLAGS) -emit-llvm -c
Review comment:
I have added llvm check before applying the patch.
```
ifeq ($(with_llvm), yes)
COMPILE.cxx.bc = $(CLANG) -xc++ -Wno-ignored-attributes $(BITCODE_CXXFLAGS) $(CPPFLAGS) -emit-llvm -c
endif
%.bc : %.cpp
$(COMPILE.cxx.bc) -o $@ $<
if [ "$(with_llvm)" = "yes" ]; then $(LLVM_BINPATH)/opt -module-summary -f $@ -o $@; fi
```
Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on pull request #28: add dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#issuecomment-660306370
> why PostgreSQL 9.6? can this be overridden easily?
PostgreSQL 9.6 is just default version. I have updated the PR to make version overridable.
Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on pull request #28: add Dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#issuecomment-671856700
The base image (debian buster) only supports LLVM 7.0 which failed to build extensions for postgresql 11+.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on a change in pull request #28: add Dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r456651085
##########
File path: Dockerfile
##########
@@ -0,0 +1,72 @@
+# 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.
+
+ARG PG_MAJOR_VERSION=9.6
+ARG PG_MINOR_VERSION=9.6
Review comment:
[PostgreSQL version policy](https://www.postgresql.org/support/versioning/) changed after 9.6, so 9.6 is also major version.
~~In order to match distribution version with docker base image, we have to separate the version.~~
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] phstudy commented on a change in pull request #28: add Dockerfile
Posted by GitBox <gi...@apache.org>.
phstudy commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r471991493
##########
File path: Dockerfile
##########
@@ -0,0 +1,72 @@
+# 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.
+
+ARG PG_MAJOR_VERSION=9.6
+ARG PG_MINOR_VERSION=9.6
Review comment:
I have made some update:
1. Default base image version was changed from `9.6` to `latest`.
2. `PG_MAJOR_VERSION` & `PG_MINOR_VERSION` arguments were removed and I parse them from pre-installed packages.
3. Only `BASE_IMAGE_VERSION` argument is remained and it is overridable, example:
```
docker build --pull --build-arg BASE_IMAGE_VERSION=10 -t datasketch-postgres:10 .
```
4. Add continuous integration with Docker on Travis CI
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [incubator-datasketches-postgresql] AlexanderSaydakov commented on a change in pull request #28: add Dockerfile
Posted by GitBox <gi...@apache.org>.
AlexanderSaydakov commented on a change in pull request #28:
URL: https://github.com/apache/incubator-datasketches-postgresql/pull/28#discussion_r476772453
##########
File path: Makefile
##########
@@ -43,6 +43,13 @@ PG_CONFIG = pg_config
PGXS := $(shell $(PG_CONFIG) --pgxs)
include $(PGXS)
+# fix LLVM JIT compilation error
+COMPILE.cxx.bc = $(CLANG) -xc++ -Wno-ignored-attributes $(BITCODE_CXXFLAGS) $(CPPFLAGS) -emit-llvm -c
Review comment:
will this work with gcc?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org