Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drush make --lock= (make-generate) strips core from version of core downloads. #1553

Closed
eric-eisenhart opened this issue Aug 14, 2015 · 11 comments

Comments

@eric-eisenhart
Copy link

eric-eisenhart commented Aug 14, 2015

Using drush 7, with a makefile like this:

api = 2
core = 7.x
projects[panopoly][type] = core

Running drush make --no-build test.make --lock=test-lock.make results in an output file with projects[panopoly][version] = "1.25".

But attempting to build with that will result in a "Could not locate panopoly version 1.25" error.

Manually editing the generated lock makefile so that version is "7.x-1.25" fixes the problem.

If I put projects[panopoly][version] = "7.x-1.25" in the original test.make makefile, it still sets version=1.25 in the output makefile.

@ergonlogic
Copy link
Member

ergonlogic commented Aug 15, 2015

I think projects[panopoly][version] = "1.25" should be valid in a makefile, since specifying core = 7.x is required anyway. So I don't think the problem is with the make-generate code that the --lock calls.

@ergonlogic
Copy link
Member

ergonlogic commented Aug 17, 2015

I've tracked this down to make_prepare_request(), where we find the following line:

$request['version'] = $type == 'core' ? $project['version'] : $project['core'] . '-' . $project['version'];

Here we're making an assumption that projects[...][type] = 'core' is going to be Drupal core, Pressflow, or some other straight drop-in, rather than a distribution.

@ergonlogic
Copy link
Member

ergonlogic commented Aug 17, 2015

This is essentially a chicken and egg problem. We need to know the project type ('project_core' or 'project_distribution') before making the request that'll provide this info.

Two obvious approaches present themselves:

(1) Add a distribution project type that Drush Make will recognize as a core; or
(2) Pull down the project XML, and check the project_type when projects[...][type] = 'core'.

I'm leaning towards (2), since only a very local change is required. Since we have (at most) one core per makefile, the additional overhead here would be minimal.

To do (1) properly, we'd probably have to figure out how to get make-generate to recognize that a site was built on a distribution.

@ergonlogic
Copy link
Member

ergonlogic commented Aug 17, 2015

@jhedstrom & @weitzman I can bang out (2) in a half-hour. However (1) is probably the "right" way to fix this. Suggestions?

@jhedstrom
Copy link
Member

jhedstrom commented Aug 17, 2015

@ergonlogic the approach in 2 sounds reasonable for the time being if it fixes the immediate problem. We can always do a follow-up if adding distribution has additional benefits.

@jonhattan
Copy link
Member

jonhattan commented Aug 17, 2015

@ergonlogic the problem is at make_generate_from_makefile() rather than make_prepare_request(). See https://github.com/drush-ops/drush/blob/master/commands/make/generate.contents.make.inc#L101

@jonhattan
Copy link
Member

jonhattan commented Aug 17, 2015

AFAICS lines 101-103 are unneccesary: $project['download']['full_version'] always contains the proper version string for Drupal core (7.38) or a distro (7.x-y.z).

@ergonlogic
Copy link
Member

ergonlogic commented Aug 17, 2015

Those lines strip the core version off of all projects, for readability. Otherwise all projects' versions will have the core prefix.

That said, the simplest fix would probably be to skip projects of type 'core' here.

@ergonlogic
Copy link
Member

ergonlogic commented Aug 17, 2015

While I still think a 'bare' version should be valid for distributions, I think that should probably be part of a more comprehensive addition of 'distribution' as a project type.

As such, I went with the simplest solution that resolves this issue. Assuming tests pass, I'd like to back-port this to Drush 7.x and 6.x. Unless there are objections.

@ergonlogic
Copy link
Member

ergonlogic commented Aug 17, 2015

Since tests passed, I'm going to back-port this trivial patch to 6.x and 7.x.

@ergonlogic
Copy link
Member

ergonlogic commented Aug 17, 2015

Cherry-picked into 7.x. The '--lock' functionality doesn't exist in Drush 6.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants