Closed Jimbolino closed 9 years ago
Hey,
First of all, thanks for the PR. However, seeing it impacts two very different features (installation and dev-*
branches), I think it'd be better as two separate PRs, if you don't mind.
Regarding the installation, the empty cache directory is definitely a good idea, however, I'm afraid the chmod 777
might create security issue on highly secure setups. For example, if an infrastructure uses ACL or other mean of securing its directories (including the cache), this might get in the way and create security issues.
Anybody able to write the cache directory might corrupt the json
returned by gitlab-composer and insert malicious references. I'd think it'd be better to create and chmod
the cache directory in a post-create-project-cmd
or post-create-project-cmd
and update the README.md
to recommend an installation through composer create-project
. This way if a sysadmin/user prefers to use an alternative security setup, they could do so easily.
dev-*
branchesReading packagist's doc (https://packagist.org/about 's Managing package versions), all branches are presented as dev-{BRANCH}
and only tags are available as plain versions, as far as I can see. If our intention is to have a behavior to packagist (for consistency purpose), I think it would be better to go this way.
Maybe using composer/composer
's strategy to handle versions could be a good idea... They can already do most of the job. I'm guessing we would only need to implement a GitLab driver (which could be based on the github one, I'm guessing). However this seems to be a major refactoring so I guess it'd be a long term option.
I'm closing this PR because I think it'd be much easier to discuss both modifications in their own PRs. Please, resubmit them separately.
Automatic cache folder chmod should make installation easier for beginners.
The other issue we had with the branches was was this: Once you create a branch that looks like a version number, eg: 2.0 And you tell composer to use "2.0" this works fine, however new commits will never be installed, because when you do "composer update", composer already sees there is a 2.0 installed, and will not update. The solution is to put "dev-2.0", however this cannot be found in the current json.
Listing all branches that look like version numbers twice (once as 2.0 and once as dev-2.0) fixed this.
Kind regards Jim