tarantool / docker

Docker images for tarantool database
https://hub.docker.com/r/tarantool/tarantool
52 stars 25 forks source link

Add curl for rocks installations #179

Closed avtikhon closed 1 year ago

avtikhon commented 4 years ago

Added curl and git packages for run time to be able to install rocks without additional dependences.

Also added curl-dev package to build time of the image. It helped to avoid of local build of the curl from sources of version 5.59.0 due to alpine OS has the following curl default packages: alpine 3.5: curl 7.61.1-r1 alpine 3.9: curl 7.64.0-r3

After build for curl from sources became unneeded, the dockerfiles alpine_3.5_2.x and alpine_3.9 became the same and were merged.

All builds for Tarantool 2.x except 2.1.0 based on alpine 3.5 version moved to use 3.9 version.

Removed installation of tarantool_curl rock from alpine_3.5_1.x dockerfile, so alpine3.5* dockerfiles became the same and were merged into alpine_3.5.

Closes #168 Part of #152

avtikhon commented 4 years ago

Hi! Thank you for the patch. For 96954ab Please, separate it into few commits (approximately the same as in your comment message.)

Ok, I'll split it a bit, but do we really want to split too much ? I'm asking it because I think that swapping/changing curl must be in a single commit. So I would suggest the following splitting on commits:

  1. Curl installation was changed from source curl to default in the OS. But changing it we needed curl-dev too not to break the builds, so it must be in a single commit:
    
    Added curl and git packages for run time to be able to install rocks
    without additional dependences.

Also added curl-dev package to build time of the image. It helped to avoid of local build of the curl from sources of version 5.59.0 due to alpine OS has the following curl default packages: alpine 3.5: curl 7.61.1-r1 alpine 3.9: curl 7.64.0-r3


2. Do we really need to split it from the 1st commit, as these was produced but changing curl and no other changes made for it ?

After build for curl from sources became unneeded, the dockerfiles alpine_3.5_2.x and alpine_3.9 became the same and were merged.

All builds for Tarantool 2.x except 2.1.0 based on alpine 3.5 version moved to use 3.9 version.


3. The following part of commit can be really split into standalone commit, but as in previous comment, does it need to be splitted into single commit or two ?

Removed installation of tarantool_curl rock from alpine_3.5_1.x dockerfile, so alpine3.5* dockerfiles became the same and were merged into alpine_3.5.

avtikhon commented 4 years ago

Hi! Thank you for the patch. For 1d3083b See comment #178 (comment)

Commented in there, please check.

LeonidVas commented 4 years ago

Hi! Thank you for the patch. For 96954ab Please, separate it into few commits (approximately the same as in your comment message.)

Ok, I'll split it a bit, but do we really want to split too much ? I'm asking it because I think that swapping/changing curl must be in a single commit. So I would suggest the following splitting on commits:

  1. Curl installation was changed from source curl to default in the OS. But changing it we needed curl-dev too not to break the builds, so it must be in a single commit:
Added curl and git packages for run time to be able to install rocks
without additional dependences.

Also added curl-dev package to build time of the image. It helped to
avoid of local build of the curl from sources of version 5.59.0 due
to alpine OS has the following curl default packages:
alpine 3.5: curl 7.61.1-r1
alpine 3.9: curl 7.64.0-r3
  1. Do we really need to split it from the 1st commit, as these was produced but changing curl and no other changes made for it ?
After build for curl from sources became unneeded, the dockerfiles
alpine_3.5_2.x and alpine_3.9 became the same and were merged.

All builds for Tarantool 2.x except 2.1.0 based on alpine 3.5 version
moved to use 3.9 version.
  1. The following part of commit can be really split into standalone commit, but as in previous comment, does it need to be splitted into single commit or two ?
Removed installation of tarantool_curl rock from alpine_3.5_1.x
dockerfile, so alpine_3.5_* dockerfiles became the same and were
merged into alpine_3.5.

Already discussed, just for a note: "You've done a nice splitting of commits at https://github.com/tarantool/docker/pull/178. I think you can repeat the same here."

@tsafin says https://github.com/tarantool/docker/pull/177 should fix the problem.

VitaliyaIoffe commented 2 years ago

I rebased it and it seems as doesn't work now

sergos commented 1 year ago

@ylobankov please proceed with review/assign

kyukhin commented 1 year ago

Alpine is going to be abandoned.