{"thread":{"id":"49679","subject":"[PATCH] travis-ci: no longer use containers","startedAt":"2018-10-25T17:42:26Z","lastAt":"2018-11-09T12:08:59Z","messageCount":9,"participants":["Sebastian Staudt","Junio C Hamano","SZEDER Gábor","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"361535","messageId":"CA+xP2SYtBGoxyV+hfjvYeEVU6XuvoZubC+-ffdubRgee=JtWXA@mail.gmail.com","threadId":"49679","inReplyTo":null,"subject":"[PATCH] travis-ci: no longer use containers","fromName":"Sebastian Staudt","fromEmail":"koraktor@gmail.com","sentAt":"2018-10-25T17:41:45Z","receivedAt":"2018-10-25T17:42:26Z","isPatch":true,"sender":{"key":"koraktor@gmail.com","avatar":"https://avatars.githubusercontent.com/u/66112?v=4"},"body":"Travis CI will soon deprecate the container-based infrastructure\nenabled by `sudo: false` in ce59dffb34190e780be2fa9f449f842cadee9753.\n\nMore info:\nhttps://blog.travis-ci.com/2018-10-04-combining-linux-infrastructures\n\nSigned-off-by: Sebastian Staudt <koraktor@gmail.com>\n---\n .travis.yml | 2 --\n 1 file changed, 2 deletions(-)\n\ndiff --git a/.travis.yml b/.travis.yml\nindex 4d4e26c9df..8d2499739e 100644\n--- a/.travis.yml\n+++ b/.travis.yml\n@@ -1,7 +1,5 @@\n language: c\n\n-sudo: false\n-\n cache:\n   directories:\n     - $HOME/travis-cache\n--\n2.19.1\n"},{"id":"361581","messageId":"xmqqo9bhfu0j.fsf@gitster-ct.c.googlers.com","threadId":"49679","inReplyTo":"CA+xP2SYtBGoxyV+hfjvYeEVU6XuvoZubC+-ffdubRgee=JtWXA@mail.gmail.com","subject":"Re: [PATCH] travis-ci: no longer use containers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-26T00:09:48Z","receivedAt":"2018-10-26T00:09:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sebastian Staudt <koraktor@gmail.com> writes:\n\n> Travis CI will soon deprecate the container-based infrastructure\n> enabled by `sudo: false` in ce59dffb34190e780be2fa9f449f842cadee9753.\n>\n> More info:\n> https://blog.travis-ci.com/2018-10-04-combining-linux-infrastructures\n\nThanks for posting a patch that would serve as a good discussion\nstarter.  This is not a criticism on your patch, but more is a RFD\nto those who helped our use of Travis by contributing to .travis.yml\nand ci/.\n\nDon't we need to do some other things so that we can run in vm\nenvironment, rather than in container environment, before doing this\nchange?  IOW, aren't we doing in .travis.yml something we can do\nonly in container but not in vm (if there is any), and if so,\nshouldn't we be rewriting that something so that we can run in vm?\n\nI know ce59dffb (\"travis-ci: explicity use container-based\ninfrastructure\", 2016-01-26) only added \"sudo: false\" without doing\nanything else (e.g. adding things that are only available to those\nwho run in container), but if we added stuff that are not usable in\nvm environment after that commit since then, we need to adjust them\nso that we can migrate to the container-based environment, no?\n\nTo me, removing that \"sudo: false\" line seems like the least thing\nwe need to worry about.  After all, they say that whether we have\n\"sudo: false\" or not, the CI jobs will start running in vm\nenvironment and not in container.  So if the rest of .travis.yml is\nready to run in vm environment, we do not have to do anything ;-).\n\nIn short, my question to Lars and SZEDER is, are we already prepared\nto be thrown into a vm environment?\n\nIf the answer is \"yes\", then I think removing \"sudo: false\" is\nprobably still a good thing to do for documentation purposes\n(i.e. showing that we knew we are ready to go through their\nmigration).\n\nThanks.\n\n> Signed-off-by: Sebastian Staudt <koraktor@gmail.com>\n> ---\n>  .travis.yml | 2 --\n>  1 file changed, 2 deletions(-)\n>\n> diff --git a/.travis.yml b/.travis.yml\n> index 4d4e26c9df..8d2499739e 100644\n> --- a/.travis.yml\n> +++ b/.travis.yml\n> @@ -1,7 +1,5 @@\n>  language: c\n>\n> -sudo: false\n> -\n>  cache:\n>    directories:\n>      - $HOME/travis-cache\n> --\n> 2.19.1\n"},{"id":"361587","messageId":"20181026013949.GN30222@szeder.dev","threadId":"49679","inReplyTo":"xmqqo9bhfu0j.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] travis-ci: no longer use containers","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-10-26T01:39:49Z","receivedAt":"2018-10-26T01:39:56Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Oct 26, 2018 at 09:09:48AM +0900, Junio C Hamano wrote:\n> Sebastian Staudt <koraktor@gmail.com> writes:\n> \n> > Travis CI will soon deprecate the container-based infrastructure\n> > enabled by `sudo: false` in ce59dffb34190e780be2fa9f449f842cadee9753.\n> >\n> > More info:\n> > https://blog.travis-ci.com/2018-10-04-combining-linux-infrastructures\n> \n> Thanks for posting a patch that would serve as a good discussion\n> starter.  This is not a criticism on your patch, but more is a RFD\n> to those who helped our use of Travis by contributing to .travis.yml\n> and ci/.\n> \n> Don't we need to do some other things so that we can run in vm\n> environment, rather than in container environment, before doing this\n> change?  IOW, aren't we doing in .travis.yml something we can do\n> only in container but not in vm (if there is any), and if so,\n> shouldn't we be rewriting that something so that we can run in vm?\n\nAs far as I understand, the container-based infrastructure has only\none benefit over the VMs, the shorter startup time.\n\nOTOH, in VMs we can use sudo, which is not available in the\ncontainer-based intra.  This has the benefit that after switching to\nVMs, we'll be able to install packages by running 'sudo apt-get\ninstall ...'.  Currently the necessary packages are listed in\n'.travis.yml' for Travis CI, while for Azure the whole install command\nis embedded in '.azure....yml'.  After the switch we could consolidate\ninstalling packages by 'sudo apt-get...' in\n'ci/install-dependencies.sh' for both.\n\n> I know ce59dffb (\"travis-ci: explicity use container-based\n> infrastructure\", 2016-01-26) only added \"sudo: false\" without doing\n> anything else (e.g. adding things that are only available to those\n> who run in container), but if we added stuff that are not usable in\n> vm environment after that commit since then, we need to adjust them\n> so that we can migrate to the container-based environment, no?\n> \n> To me, removing that \"sudo: false\" line seems like the least thing\n> we need to worry about.  After all, they say that whether we have\n> \"sudo: false\" or not, the CI jobs will start running in vm\n> environment and not in container.  So if the rest of .travis.yml is\n> ready to run in vm environment, we do not have to do anything ;-).\n> \n> In short, my question to Lars and SZEDER is, are we already prepared\n> to be thrown into a vm environment?\n\nI think we are.  I've run only two builds with this patch, and they\nrun smoothly and finished successfully.  After you update 'pu' I'll\nrun more.\n\n> If the answer is \"yes\", then I think removing \"sudo: false\" is\n> probably still a good thing to do for documentation purposes\n> (i.e. showing that we knew we are ready to go through their\n> migration).\n\nI agree.\n\n\n> > Signed-off-by: Sebastian Staudt <koraktor@gmail.com>\n> > ---\n> >  .travis.yml | 2 --\n> >  1 file changed, 2 deletions(-)\n> >\n> > diff --git a/.travis.yml b/.travis.yml\n> > index 4d4e26c9df..8d2499739e 100644\n> > --- a/.travis.yml\n> > +++ b/.travis.yml\n> > @@ -1,7 +1,5 @@\n> >  language: c\n> >\n> > -sudo: false\n> > -\n> >  cache:\n> >    directories:\n> >      - $HOME/travis-cache\n> > --\n> > 2.19.1\n"},{"id":"361598","messageId":"CA+xP2Sbm7YGU3tKMNEzrvZfOQScgE2-ap1GVCXcjLH5botr4ug@mail.gmail.com","threadId":"49679","inReplyTo":"xmqqo9bhfu0j.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] travis-ci: no longer use containers","fromName":"Sebastian Staudt","fromEmail":"koraktor@gmail.com","sentAt":"2018-10-26T05:06:18Z","receivedAt":"2018-10-26T05:07:00Z","isPatch":true,"sender":{"key":"koraktor@gmail.com","avatar":"https://avatars.githubusercontent.com/u/66112?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n>\n> Sebastian Staudt <koraktor@gmail.com> writes:\n>\n> > Travis CI will soon deprecate the container-based infrastructure\n> > enabled by `sudo: false` in ce59dffb34190e780be2fa9f449f842cadee9753.\n> >\n> > More info:\n> > https://blog.travis-ci.com/2018-10-04-combining-linux-infrastructures\n>\n> Thanks for posting a patch that would serve as a good discussion\n> starter.  This is not a criticism on your patch, but more is a RFD\n> to those who helped our use of Travis by contributing to .travis.yml\n> and ci/.\n\nIn fact this was the intention while creating this patch. Although I see\nI could have made this a bit clearer in the initial message.\n\nHaving a patch that may cause a broken build or other CI problems\nseems more appropriate than waiting for Travis CI to flip the switch\nand searching for the problem afterwards.\n\n> Don't we need to do some other things so that we can run in vm\n> environment, rather than in container environment, before doing this\n> change?  IOW, aren't we doing in .travis.yml something we can do\n> only in container but not in vm (if there is any), and if so,\n> shouldn't we be rewriting that something so that we can run in vm?\n>\n> I know ce59dffb (\"travis-ci: explicity use container-based\n> infrastructure\", 2016-01-26) only added \"sudo: false\" without doing\n> anything else (e.g. adding things that are only available to those\n> who run in container), but if we added stuff that are not usable in\n> vm environment after that commit since then, we need to adjust them\n> so that we can migrate to the container-based environment, no?\n>\n> To me, removing that \"sudo: false\" line seems like the least thing\n> we need to worry about.  After all, they say that whether we have\n> \"sudo: false\" or not, the CI jobs will start running in vm\n> environment and not in container.  So if the rest of .travis.yml is\n> ready to run in vm environment, we do not have to do anything ;-).\n>\n> In short, my question to Lars and SZEDER is, are we already prepared\n> to be thrown into a vm environment?\n>\n> If the answer is \"yes\", then I think removing \"sudo: false\" is\n> probably still a good thing to do for documentation purposes\n> (i.e. showing that we knew we are ready to go through their\n> migration).\n>\n> Thanks.\n>\n> > Signed-off-by: Sebastian Staudt <koraktor@gmail.com>\n> > ---\n> >  .travis.yml | 2 --\n> >  1 file changed, 2 deletions(-)\n> >\n> > diff --git a/.travis.yml b/.travis.yml\n> > index 4d4e26c9df..8d2499739e 100644\n> > --- a/.travis.yml\n> > +++ b/.travis.yml\n> > @@ -1,7 +1,5 @@\n> >  language: c\n> >\n> > -sudo: false\n> > -\n> >  cache:\n> >    directories:\n> >      - $HOME/travis-cache\n> > --\n> > 2.19.1\n"},{"id":"362164","messageId":"20181101114714.14710-1-szeder.dev@gmail.com","threadId":"49679","inReplyTo":"20181026013949.GN30222@szeder.dev","subject":"[PATCH] travis-ci: install packages in 'ci/install-dependencies.sh'","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-11-01T11:47:14Z","receivedAt":"2018-11-01T11:47:29Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Ever since we started using Travis CI, we specified the list of\npackages to install in '.travis.yml' via the APT addon.  While running\nour builds on Travis CI's container-based infrastructure we didn't\nhave another choice, because that environment didn't support 'sudo',\nand thus we didn't have permission to install packages ourselves.  With\nthe switch to the VM-based infrastructure in the previous patch we do\nget a working 'sudo', so we can install packages by running 'sudo\napt-get -y install ...' as well.\n\nLet's make use of this and install necessary packages in\n'ci/install-dependencies.sh', so all the dependencies (i.e. both\npackages and \"non-packages\" (P4 and Git-LFS)) are handled in the same\nfile.  Install gcc-8 only in the 'linux-gcc' build job; so far it has\nbeen unnecessarily installed in the 'linux-clang' build job as well.\nPrint the versions of P4 and Git-LFS conditionally, i.e. only when\nthey have been installed; with this change even the static analysis\nand documentation build jobs start using 'ci/install-dependencies.sh'\nto install packages, and neither of these two build jobs depend on and\nthus install those.\n\nThis change will presumably be beneficial for the upcoming Azure\nPipelines integration [1]: preliminary versions of that patch series\nrun a couple of 'apt-get' commands to install the necessary packages\nbefore running 'ci/install-dependencies.sh', but with this patch it\nwill be sufficient to run only 'ci/install-dependencies.sh'.\n\n[1] https://public-inbox.org/git/1a22efe849d6da79f2c639c62a1483361a130238.1539598316.git.gitgitgadget@gmail.com/\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n\nThis patch should go on top of 'ss/travis-ci-force-vm-mode'.\n\nI'm not sure about the last paragraph, because:\n\n  - It talks about presumed benefits for a currently still\n    work-in-progress patch series of an other contributor, and I'm not\n    really sure that that's a good thing.  Perhaps I should have\n    rather put it below the '---'.\n\n  - I'm confused about the name of this Azure thing.  The cover letter\n    mentions \"Azure Pipelines\", the file is called\n    'azure-pipelines.yml', but the relevant patch I link to talks\n    about \"Azure DevOps\" in the commit message.\n\nAnyway, keep that last paragraph or drop it as you see fit.\n\n\n .travis.yml                | 21 ---------------------\n ci/install-dependencies.sh | 35 +++++++++++++++++++++++++++++------\n 2 files changed, 29 insertions(+), 27 deletions(-)\n\ndiff --git a/.travis.yml b/.travis.yml\nindex 8d2499739e..a5a82d6832 100644\n--- a/.travis.yml\n+++ b/.travis.yml\n@@ -12,16 +12,6 @@ compiler:\n   - clang\n   - gcc\n \n-addons:\n-  apt:\n-    sources:\n-    - ubuntu-toolchain-r-test\n-    packages:\n-    - language-pack-is\n-    - git-svn\n-    - apache2\n-    - gcc-8\n-\n matrix:\n   include:\n     - env: jobname=GETTEXT_POISON\n@@ -50,22 +40,11 @@ matrix:\n     - env: jobname=StaticAnalysis\n       os: linux\n       compiler:\n-      addons:\n-        apt:\n-          packages:\n-          - coccinelle\n-      before_install:\n       script: ci/run-static-analysis.sh\n       after_failure:\n     - env: jobname=Documentation\n       os: linux\n       compiler:\n-      addons:\n-        apt:\n-          packages:\n-          - asciidoc\n-          - xmlto\n-      before_install:\n       script: ci/test-documentation.sh\n       after_failure:\n \ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex 75a9fd2475..06c3546e1e 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -10,6 +10,15 @@ LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VE\n \n case \"$jobname\" in\n linux-clang|linux-gcc)\n+\tsudo apt-add-repository -y \"ppa:ubuntu-toolchain-r/test\"\n+\tsudo apt-get -q update\n+\tsudo apt-get -q -y install language-pack-is git-svn apache2\n+\tcase \"$jobname\" in\n+\tlinux-gcc)\n+\t\tsudo apt-get -q -y install gcc-8\n+\t\t;;\n+\tesac\n+\n \tmkdir --parents \"$P4_PATH\"\n \tpushd \"$P4_PATH\"\n \t\twget --quiet \"$P4WHENCE/bin.linux26x86_64/p4d\"\n@@ -32,11 +41,25 @@ osx-clang|osx-gcc)\n \tbrew link --force gettext\n \tbrew install caskroom/cask/perforce\n \t;;\n+StaticAnalysis)\n+\tsudo apt-get -q update\n+\tsudo apt-get -q -y install coccinelle\n+\t;;\n+Documentation)\n+\tsudo apt-get -q update\n+\tsudo apt-get -q -y install asciidoc xmlto\n+\t;;\n esac\n \n-echo \"$(tput setaf 6)Perforce Server Version$(tput sgr0)\"\n-p4d -V | grep Rev.\n-echo \"$(tput setaf 6)Perforce Client Version$(tput sgr0)\"\n-p4 -V | grep Rev.\n-echo \"$(tput setaf 6)Git-LFS Version$(tput sgr0)\"\n-git-lfs version\n+if type p4d >/dev/null && type p4 >/dev/null\n+then\n+\techo \"$(tput setaf 6)Perforce Server Version$(tput sgr0)\"\n+\tp4d -V | grep Rev.\n+\techo \"$(tput setaf 6)Perforce Client Version$(tput sgr0)\"\n+\tp4 -V | grep Rev.\n+fi\n+if type git-lfs >/dev/null\n+then\n+\techo \"$(tput setaf 6)Git-LFS Version$(tput sgr0)\"\n+\tgit-lfs version\n+fi\n-- \n2.19.1.838.g101e68a972\n\n"},{"id":"362227","messageId":"xmqqa7msrzaq.fsf@gitster-ct.c.googlers.com","threadId":"49679","inReplyTo":"20181101114714.14710-1-szeder.dev@gmail.com","subject":"Re: [PATCH] travis-ci: install packages in 'ci/install-dependencies.sh'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-02T02:25:17Z","receivedAt":"2018-11-02T02:25:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder.dev@gmail.com> writes:\n\n> Ever since we started using Travis CI, we specified the list of\n> packages to install in '.travis.yml' via the APT addon.  While running\n> our builds on Travis CI's container-based infrastructure we didn't\n> have another choice, because that environment didn't support 'sudo',\n> and thus we didn't have permission to install packages ourselves.  With\n> the switch to the VM-based infrastructure in the previous patch we do\n> get a working 'sudo', so we can install packages by running 'sudo\n> apt-get -y install ...' as well.\n\nOK, so far we learned that this is now _doable_; but not enough to\ndecide if this is a good thing to do or not.  Let's read on to find\nout.\n\n> Let's make use of this and install necessary packages in\n> 'ci/install-dependencies.sh', so all the dependencies (i.e. both\n> packages and \"non-packages\" (P4 and Git-LFS)) are handled in the same\n> file.  \n\nSo we used to have two waysto prepare the test environment; non\npackaged software were done via install-dependencies.sh, but\npackaged ones weren't.  Unifying them so that the script installs\nboth would be a good change to simplify the procedure.  \n\nIs that how this sentence argues for this change?\n\n> Install gcc-8 only in the 'linux-gcc' build job; so far it has\n> been unnecessarily installed in the 'linux-clang' build job as well.\n\nIs this \"unneeded gcc-8 was installed\" something we can fix only\nbecause we now stopped doing the installation via apt addon?  Or we\ncould have fixed it while we were on apt addon but we didn't bother,\nand this patch fixes it \"while at it\"---primarily because the shell\nscript is far more flexible to work with than travis.yml matrix and\nthis kind of customization is far easier to do?\n\n> Print the versions of P4 and Git-LFS conditionally, i.e. only when\n> they have been installed; with this change even the static analysis\n> and documentation build jobs start using 'ci/install-dependencies.sh'\n> to install packages, and neither of these two build jobs depend on and\n> thus install those.\n>\n> This change will presumably be beneficial for the upcoming Azure\n> Pipelines integration [1]: preliminary versions of that patch series\n> run a couple of 'apt-get' commands to install the necessary packages\n> before running 'ci/install-dependencies.sh', but with this patch it\n> will be sufficient to run only 'ci/install-dependencies.sh'.\n\nSo the main point of this change is to have less knowledge to\nprepare the target configuration in the .travis.yml file and keep\nthem all in ci/install-dependencies.sh, which hopefully is more\nreusable than .travis.yml in a non Travis environment?\n\nIf that is the case, it makes sense to me.\n\n> This patch should go on top of 'ss/travis-ci-force-vm-mode'.\n>\n> I'm not sure about the last paragraph, because:\n>\n>   - It talks about presumed benefits for a currently still\n>     work-in-progress patch series of an other contributor, and I'm not\n>     really sure that that's a good thing.  Perhaps I should have\n>     rather put it below the '---'.\n>\n>   - I'm confused about the name of this Azure thing.  The cover letter\n>     mentions \"Azure Pipelines\", the file is called\n>     'azure-pipelines.yml', but the relevant patch I link to talks\n>     about \"Azure DevOps\" in the commit message.\n>\n> Anyway, keep that last paragraph or drop it as you see fit.\n\nI hope we'll hear from Dscho in one or two revolutions of the Earth\n;-)\n\n> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\n> index 75a9fd2475..06c3546e1e 100755\n> --- a/ci/install-dependencies.sh\n> +++ b/ci/install-dependencies.sh\n> @@ -10,6 +10,15 @@ LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VE\n>  \n>  case \"$jobname\" in\n>  linux-clang|linux-gcc)\n> +\tsudo apt-add-repository -y \"ppa:ubuntu-toolchain-r/test\"\n> +\tsudo apt-get -q update\n> +\tsudo apt-get -q -y install language-pack-is git-svn apache2\n> +\tcase \"$jobname\" in\n> +\tlinux-gcc)\n> +\t\tsudo apt-get -q -y install gcc-8\n> +\t\t;;\n> +\tesac\n> +\n>  \tmkdir --parents \"$P4_PATH\"\n>  \tpushd \"$P4_PATH\"\n>  \t\twget --quiet \"$P4WHENCE/bin.linux26x86_64/p4d\"\n> @@ -32,11 +41,25 @@ osx-clang|osx-gcc)\n>  \tbrew link --force gettext\n>  \tbrew install caskroom/cask/perforce\n>  \t;;\n> +StaticAnalysis)\n> +\tsudo apt-get -q update\n> +\tsudo apt-get -q -y install coccinelle\n> +\t;;\n> +Documentation)\n> +\tsudo apt-get -q update\n> +\tsudo apt-get -q -y install asciidoc xmlto\n> +\t;;\n>  esac\n>  \n> -echo \"$(tput setaf 6)Perforce Server Version$(tput sgr0)\"\n> -p4d -V | grep Rev.\n> -echo \"$(tput setaf 6)Perforce Client Version$(tput sgr0)\"\n> -p4 -V | grep Rev.\n> -echo \"$(tput setaf 6)Git-LFS Version$(tput sgr0)\"\n> -git-lfs version\n> +if type p4d >/dev/null && type p4 >/dev/null\n> +then\n> +\techo \"$(tput setaf 6)Perforce Server Version$(tput sgr0)\"\n> +\tp4d -V | grep Rev.\n> +\techo \"$(tput setaf 6)Perforce Client Version$(tput sgr0)\"\n> +\tp4 -V | grep Rev.\n> +fi\n> +if type git-lfs >/dev/null\n> +then\n> +\techo \"$(tput setaf 6)Git-LFS Version$(tput sgr0)\"\n> +\tgit-lfs version\n> +fi\n"},{"id":"362759","messageId":"20181108215133.GC30222@szeder.dev","threadId":"49679","inReplyTo":"xmqqa7msrzaq.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] travis-ci: install packages in 'ci/install-dependencies.sh'","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-11-08T21:51:33Z","receivedAt":"2018-11-08T21:51:41Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Nov 02, 2018 at 11:25:17AM +0900, Junio C Hamano wrote:\n> SZEDER Gábor <szeder.dev@gmail.com> writes:\n> \n> > Ever since we started using Travis CI, we specified the list of\n> > packages to install in '.travis.yml' via the APT addon.  While running\n> > our builds on Travis CI's container-based infrastructure we didn't\n> > have another choice, because that environment didn't support 'sudo',\n> > and thus we didn't have permission to install packages ourselves.  With\n> > the switch to the VM-based infrastructure in the previous patch we do\n> > get a working 'sudo', so we can install packages by running 'sudo\n> > apt-get -y install ...' as well.\n> \n> OK, so far we learned that this is now _doable_; but not enough to\n> decide if this is a good thing to do or not.  Let's read on to find\n> out.\n\nYeah, this paragraph is just a bit of background about how the current\nsituation came to be and what recent change made the switch possible.\n\n> > Let's make use of this and install necessary packages in\n> > 'ci/install-dependencies.sh', so all the dependencies (i.e. both\n> > packages and \"non-packages\" (P4 and Git-LFS)) are handled in the same\n> > file.  \n> \n> So we used to have two waysto prepare the test environment; non\n> packaged software were done via install-dependencies.sh, but\n> packaged ones weren't.  Unifying them so that the script installs\n> both would be a good change to simplify the procedure.  \n> \n> Is that how this sentence argues for this change?\n\nYes.\n\n> > Install gcc-8 only in the 'linux-gcc' build job; so far it has\n> > been unnecessarily installed in the 'linux-clang' build job as well.\n> \n> Is this \"unneeded gcc-8 was installed\" something we can fix only\n> because we now stopped doing the installation via apt addon?\n\nNow that you mention it: no.  It would have been possible to install\ngcc-8 only in the 'linux-gcc' build job even via the apt addon, namely\nby removing the two Linux build jobs from the implicit build matrix\nand adding them as two independent build jobs in the 'matrix.include'\nsection of '.travis.yml'.  The drawback is that all the extra packages\nused in both build jobs would have to be duplicated.\n\n> Or we\n> could have fixed it while we were on apt addon but we didn't bother,\n> and this patch fixes it \"while at it\"---primarily because the shell\n> script is far more flexible to work with than travis.yml matrix and\n> this kind of customization is far easier to do?\n\nBasically yes (though I think it's not about not bothering; I don't\nknow about others, but it just occured to me that it would have been\ndoable, however, even if it occured to me earlier, because of the\nduplicated list of common packages I wouldn't have done it).\n\nDoing it in good old shell is indeed easier and the common packages\nare then only listed once.\n\n> > Print the versions of P4 and Git-LFS conditionally, i.e. only when\n> > they have been installed; with this change even the static analysis\n> > and documentation build jobs start using 'ci/install-dependencies.sh'\n> > to install packages, and neither of these two build jobs depend on and\n> > thus install those.\n> >\n> > This change will presumably be beneficial for the upcoming Azure\n> > Pipelines integration [1]: preliminary versions of that patch series\n> > run a couple of 'apt-get' commands to install the necessary packages\n> > before running 'ci/install-dependencies.sh', but with this patch it\n> > will be sufficient to run only 'ci/install-dependencies.sh'.\n> \n> So the main point of this change is to have less knowledge to\n> prepare the target configuration in the .travis.yml file and keep\n> them all in ci/install-dependencies.sh, which hopefully is more\n> reusable than .travis.yml in a non Travis environment?\n\nOh, \"more reusable\" indeed, that's a more eloquent way to put it.\n\n> If that is the case, it makes sense to me.\n> \n> > This patch should go on top of 'ss/travis-ci-force-vm-mode'.\n> >\n> > I'm not sure about the last paragraph, because:\n> >\n> >   - It talks about presumed benefits for a currently still\n> >     work-in-progress patch series of an other contributor, and I'm not\n> >     really sure that that's a good thing.  Perhaps I should have\n> >     rather put it below the '---'.\n> >\n> >   - I'm confused about the name of this Azure thing.  The cover letter\n> >     mentions \"Azure Pipelines\", the file is called\n> >     'azure-pipelines.yml', but the relevant patch I link to talks\n> >     about \"Azure DevOps\" in the commit message.\n> >\n> > Anyway, keep that last paragraph or drop it as you see fit.\n> \n> I hope we'll hear from Dscho in one or two revolutions of the Earth\n> ;-)\n\n... revolutions around what? :)\n\n"},{"id":"362770","messageId":"xmqqr2fv5528.fsf@gitster-ct.c.googlers.com","threadId":"49679","inReplyTo":"20181108215133.GC30222@szeder.dev","subject":"Re: [PATCH] travis-ci: install packages in 'ci/install-dependencies.sh'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-09T02:59:43Z","receivedAt":"2018-11-09T02:59:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder.dev@gmail.com> writes:\n\n>> > I'm not sure about the last paragraph, because:\n>> >\n>> >   - It talks about presumed benefits for a currently still\n>> >     work-in-progress patch series of an other contributor, and I'm not\n>> >     really sure that that's a good thing.  Perhaps I should have\n>> >     rather put it below the '---'.\n>> >\n>> >   - I'm confused about the name of this Azure thing.  The cover letter\n>> >     mentions \"Azure Pipelines\", the file is called\n>> >     'azure-pipelines.yml', but the relevant patch I link to talks\n>> >     about \"Azure DevOps\" in the commit message.\n>> >\n>> > Anyway, keep that last paragraph or drop it as you see fit.\n>> \n>> I hope we'll hear from Dscho in one or two revolutions of the Earth\n>> ;-)\n>\n> ... revolutions around what? :)\n\nOriginally I meant its own axis, but perhaps the moon.\n\n\n"},{"id":"362822","messageId":"nycvar.QRO.7.76.6.1811091305550.39@tvgsbejvaqbjf.bet","threadId":"49679","inReplyTo":"xmqqr2fv5528.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] travis-ci: install packages in 'ci/install-dependencies.sh'","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-09T12:08:51Z","receivedAt":"2018-11-09T12:08:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 9 Nov 2018, Junio C Hamano wrote:\n\n> SZEDER Gábor <szeder.dev@gmail.com> writes:\n> \n> >> > I'm not sure about the last paragraph, because:\n> >> >\n> >> >   - It talks about presumed benefits for a currently still\n> >> >     work-in-progress patch series of an other contributor, and I'm not\n> >> >     really sure that that's a good thing.  Perhaps I should have\n> >> >     rather put it below the '---'.\n> >> >\n> >> >   - I'm confused about the name of this Azure thing.  The cover letter\n> >> >     mentions \"Azure Pipelines\", the file is called\n> >> >     'azure-pipelines.yml', but the relevant patch I link to talks\n> >> >     about \"Azure DevOps\" in the commit message.\n> >> >\n> >> > Anyway, keep that last paragraph or drop it as you see fit.\n> >> \n> >> I hope we'll hear from Dscho in one or two revolutions of the Earth\n> >> ;-)\n> >\n> > ... revolutions around what? :)\n> \n> Originally I meant its own axis, but perhaps the moon.\n\nI see, you had fun talking about a revolution [*1*]...\n\nI am much in favor of this patch, as it indeed will simplify the\nintegration into Azure Pipelines.\n\nAs to the DevOps vs Pipelines thing: DevOps is the umbrella, it consists\nof much more than just the automated builds (Pipelines), it also has\nBoards, Packages, etc, but I don't think we will ever use that in the Git\nproject, as our workflow is based out of a mailing list.\n\nCiao,\nDscho\n\n\nFootnote *1*: https://www.youtube.com/watch?v=Xv8FBjo1Y8I"}]}