fix(Dockerfile): solve composer usage #731

bcornec · 2017-01-05T21:30:50Z

Adapt Dockerfile to the new place where composer is
downloaded and used.

Adapt Dockerfile to the new place where composer is downloaded and used.

heckj · 2017-01-05T23:10:20Z

Thanks @bcornec

- Only need EPEL repo for external deps - Requires file-devel at build time

- Fix package name creation for RPM based distros - Plan for more Debian/Ubuntu generation

FrenchBen · 2017-01-11T19:53:08Z

Patch works for me and fixes #732 👍

maxhbr · 2017-01-18T15:17:06Z

Dockerfile

 RUN curl -sS https://getcomposer.org/installer | php && \
-    mv composer.phar /usr/local/bin/composer
+	mkdir /tmp/composer && \
+    mv composer.phar /tmp/composer/composer


the path /tmp/composer/composer is not a very good choice, since

this folder gets cleaned on every reboot on most systems

/tmp/ might be mounted with noexec for security reasons

it might interfere with other fossoloy development setups

Note: this comment is to late and is related to #701 which is already merged. In my opinion one should change COMPOSERTMP = /tmp/composer to something in the current subtree.

The script fo_installdeps still installs composer to the "old" place:

fossology/utils/fo-installdeps

Line 267 in 35d9576

curl -sS https://getcomposer.org/installer | php && mv composer.phar /usr/local/bin/composer

If one calls the following

$ make composer_download $ sudo make composer_install

one is no longer able to execute composer_download again, since root owns some of the files in ~/.composer. Thus loosing the binary at /tmp/composer/composer is hard problem. (tested on vagrant setup)

screenshot:

Idea: on Projectbuilder setup install composer to /tmp/composer/composer and add /tmp/composer to $PATH before calling make phpvendors resp make composer_install and remove all changes related to the placement of composer.

@bcornec would this be ok for you?

You points wrt noexec are valid so I agree we should find a better place to host it.
I didn't want to perturbate your process too much, but indeed downloading it in the fossology tree should bring what we need and solve the issues you mentioned. I'll propose a new patch for that.

This does not solve the problem with "once you have used sudo make composer_install and you have lost the composer binary, you are no longer able to install composer via make composer_download" (unless you use admin privileges remove ~/.composer) see comment above.

What speaks against using the $PATH variable to tell the makefiles where to look for composer?

Using PATH is relying on environment to be correct in the first place. If another command is named composer and placed earlier in PATH, you lost :-( And if it's a malicious one, it's worse.

What's wrong with calling it with the full path name, once you know where it should be hosted ?

I think the approach we currently have is the right one, except for the location which should be under the fossology tree (which I didn't want to do myself). Also which variable can be used to declare that fossology tree ?

Wrt the .composer dir, I wasn't aware of that. But the solution is rather to remove that directory then in the Makefile at last step of the install phase (run by root as well), as anyway it shouldn't be created as root.

But:

we still do not know where it should be hosted

the command make composer_download fails after composer was called once as root

you need to download it once for every source tree

if you clone in a new location you might not be able to install the necessary composer, unless you remove the ~/.composer folder. In fact you might not have the necessary permissions to do that and thus are not able build the project

being forced to remove the folder ~/.composer is not good

you then have to adopt

the fo-installdepts script

the documentation: an additional step is needed for the source installation (the archived documentation which is still used by some people and is easily found will stop working)

the docker scripts

the vagrant scripts

Using PATH is relying on environment to be correct in the first place.

since you are the person defining PATH=/my/composer/path:$PATH that should not be a problem

you might not want to prevent users from using system packages of composer. E.g. my operating system offers them.

Also which variable can be used to declare that fossology tree ?

Do you search for an environmental variable used for this? I'm not aware of such an variable in the Makefiles

But the solution is rather to remove that directory then in the Makefile at last step of the install phase (run by root as well)

You can't simply remove the ~/.composer folder, which might be used for other projects on the machine.

... as anyway it shouldn't be created as root.

That is an composer problem. When downloaded and installed as user, it seems to write the current home into the composer.phar which we rename to composer. Thus runnig this composer with root touches files in ~/.composer of the user.

this commit - undos the changes to the composer placement introduced in fossology#701 - adds the wrapper script `utils/run_with_composer_in_path.sh` which downloads composer on demand and adds it to the PATH for the passed command. (usage: `path/to/utils/run_with_composer_in_path.sh make composer_install`) - this fixes the source installation, i.e. the documentation was not up to date - this fixes the problems with `vagrant up` (see fossology#732) - this fixes the problems with `docker-compose build` as discussed in fossology#731 closes fossology#732

maxhbr · 2017-02-01T13:07:38Z

See #738 as alternative solution.

Fixes issue where a pointer to a local variable is being returned from the `getRegexValue` function, causing an invalid regex to be passed to the copyright agent, triggering an infinite loop and agent crash after memory exhaustion.

As a quickfix, this adds a fallback in `phpunit-bootstrap.php` which will work in all cases.

Cleaning up the final version of the vagrant test file in pbconf for testing and developing package builds after it worked for centos7. In addition fixing configuration for Apache httpd 2.4

Download composer into a fossology subdir (tmp) setup COMPOSER_HOME to avoid creation of a .composer dir at install Fix Docker and Vagrant to use make composer_install

this commit - undos the changes to the composer placement introduced in fossology#701 - adds the option to pass the environmental variable `COMPOSER_PHAR` to `src/Makefile` to allow placing the composer.phar at arbitrary places - this fixes the source installation, i.e. the documentation was not up to date - this fixes the problems with `vagrant up` (see fossology#732) - this fixes the problems with `docker-compose build` as discussed in fossology#731 closes fossology#732

bcornec · 2017-02-01T18:08:40Z

Replacing with #739 for composer and #740 for project-builder.org patches

this commit - undos the changes to the composer placement introduced in fossology#701 - adds the option to pass the environmental variable `COMPOSER_PHAR` to `src/Makefile` to allow placing the composer.phar at arbitrary places - this fixes the source installation, i.e. the documentation was not up to date - this fixes the problems with `vagrant up` (see fossology#732) - this fixes the problems with `docker-compose build` as discussed in fossology#731 closes fossology#732

Fix(Dockerfile): solve composer usage

8871fef

Adapt Dockerfile to the new place where composer is downloaded and used.

bcornec mentioned this pull request Jan 5, 2017

Source Code install of 3.1.0-rc2 on Debian 8 fails for composer error #690

Closed

bcornec added 2 commits January 7, 2017 01:41

Fix CentOS 7 build

ccae2d3

- Only need EPEL repo for external deps - Requires file-devel at build time

More project-builder improvements

35d9576

- Fix package name creation for RPM based distros - Plan for more Debian/Ubuntu generation

bcornec mentioned this pull request Jan 11, 2017

docker-compose up fails to build fossology-scheduler #732

Closed

maxhbr requested changes Jan 18, 2017

View reviewed changes

maxhbr mentioned this pull request Feb 1, 2017

fix(make): do not place composer at /tmp/composer/composer #738

Merged

timurphy and others added 6 commits February 1, 2017 17:08

fix(copyright): invalid pointer to regex

cb24071

Fixes issue where a pointer to a local variable is being returned from the `getRegexValue` function, causing an invalid regex to be passed to the copyright agent, triggering an infinite loop and agent crash after memory exhaustion.

fix(install): xenial support for postgres

17ce3d7

Fix docker usage information

0f3d3c4

fix(test): phpunit-bootstrap doesn't find Hamcrest

c7c97c1

As a quickfix, this adds a fallback in `phpunit-bootstrap.php` which will work in all cases.

feat(packaging): vagrant test file and config for httpd 2.4

9464c40

Cleaning up the final version of the vagrant test file in pbconf for testing and developing package builds after it worked for centos7. In addition fixing configuration for Apache httpd 2.4

Improve composer handling

7d0c4ba

Download composer into a fossology subdir (tmp) setup COMPOSER_HOME to avoid creation of a .composer dir at install Fix Docker and Vagrant to use make composer_install

bcornec mentioned this pull request Feb 1, 2017

Improve composer handling #739

Closed

bcornec mentioned this pull request Feb 1, 2017

fix(Project-builder.org): various enhancements #740

Merged

bcornec closed this Feb 1, 2017

fix(Dockerfile): solve composer usage #731

fix(Dockerfile): solve composer usage #731

Uh oh!

Conversation

bcornec commented Jan 5, 2017

Uh oh!

heckj commented Jan 5, 2017

Uh oh!

FrenchBen commented Jan 11, 2017

Uh oh!

maxhbr Jan 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maxhbr Jan 18, 2017

Choose a reason for hiding this comment

Uh oh!

maxhbr Jan 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maxhbr Jan 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bcornec Jan 26, 2017

Choose a reason for hiding this comment

Uh oh!

maxhbr Jan 26, 2017

Choose a reason for hiding this comment

Uh oh!

bcornec Feb 1, 2017

Choose a reason for hiding this comment

Uh oh!

maxhbr Feb 1, 2017

Choose a reason for hiding this comment

Uh oh!

maxhbr commented Feb 1, 2017

Uh oh!

bcornec commented Feb 1, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

maxhbr Jan 18, 2017 •

edited

Loading

maxhbr Jan 18, 2017 •

edited

Loading

maxhbr Jan 18, 2017 •

edited

Loading