Drush Make + Undefined Translations [#1543774]

When adding translations = "fr" to a drush make file everything works splendidly. However if some modules for example apachesolr_attachments don't have a translation the drush make still tries to pull down the file. This results in a translations folder and file being created (fr.po) with the following contents.

<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>404 Not Found</title>
</head><body>
<h1>Not Found</h1>
<p>The requested URL /files/translations/7.x/apachesolr_attachments/apachesolr_attachments-7.x-1.x-dev.fr.po was not found on this server.</p>
<hr>
<address>Apache Server at ftp.drupal.org Port 80</address>
</body></html>

Is it possible to not create the translations folder with drush make should the translation itself not exist?

Comment	File	Size	Author
#5	Drush_Make_Undefined_Translations-1543774-5.patch	657 bytes	helmo

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

sylus CreditAttribution: sylus commented 23 April 2012 at 19:30

Version:

» 7.x-5.1

Should mention tested this with windows using msysgit.

Comment #2

sylus CreditAttribution: sylus commented 2 May 2012 at 13:41

This problem still exists and is a duplicate of: http://drupal.org/node/997996

Comment #3

sylus CreditAttribution: sylus commented 16 May 2012 at 20:09

Version:

7.x-5.1

Are we able to apply the patch from http://drupal.org/node/997996.

Would be awesome to have Drush Make working with translations fully. I think all we have to do is

- elseif ($download['request_type'] == 'post' && drush_shell_cd_and_exec($download_path, 'curl -d %s -LOD %s %s', $download['data'], $header_file, $url)) {
+ elseif ($download['request_type'] == 'post' && drush_shell_cd_and_exec($download_path, 'curl -f -d %s -LOD %s %s', $download['data'], $header_file, $url)) {

Basically just adding curl -f -d.

Comment #4

clemens.tolboom

Dutch

Groningen, 🇳🇱/🇪🇺

CreditAttribution: clemens.tolboom commented 22 May 2012 at 09:35

Running the following gives me 8 hits :(

find . -name *.po -exec echo {} \; -exec grep DOCTYPE {} \; | grep DOCTYPE

Comment #5

helmo CreditAttribution: helmo commented 22 May 2012 at 12:44

Status:

Active

» Needs review

File	Size
Drush_Make_Undefined_Translations-1543774-5.patch	657 bytes

I managed to reproduce this.

I had to disable wget as that is the first choice for _drush_download_file(), clemens.tolboom has a system where no wget is installed.

As I generally prefer to program long-opts I've added --fail to the curl command.

What do the test bots say about this patch?

Comment #6

moshe weitzman CreditAttribution: moshe weitzman commented 22 May 2012 at 14:19

_drush_download_file is used for dl command and maybe others. i don't know that adding --fail there makes sense for all user cases. it makes sense for the OP though.

Comment #7

helmo CreditAttribution: helmo commented 23 May 2012 at 06:36

I was hoping that someone could run the testsuite with this patch to check for regressions.

I've tried travis-ci, but the tests are failing there. (http://travis-ci.org/#!/helmo/drush and http://travis-ci.org/#!/drush-ops/drush)

Comment #8

sylus CreditAttribution: sylus commented 23 May 2012 at 19:21

Used this patch with Drush and it did fix all of my problems. Noticed no other problems. Definitely interested if there are any regressions with this patch. Hopefully there is not and it can be committed.

It is interesting to note that this will always be a problem for windows users as even with the "wget" library being added to the "propeople drush" package the curl function will always be called. This is because drush checks for wget using the "which" command which itself is not part of the package so resorts to curl as a failsafe.

This would also fail for mac os x as wget is not installed by default.

Obviously running the drush make on a linux server works since wget is present.

Comment #9

moshe weitzman CreditAttribution: moshe weitzman commented 25 May 2012 at 14:32

Status:

Needs review

» Needs work

Setting to Needs work for #6

Comment #10

helmo CreditAttribution: helmo commented 27 May 2012 at 12:47

@moshe weitzman: could you run the testsuite after applying this patch? Or advise how I can get a complete testrun on travis-ci or some other standardised platform?

Comment #11

helmo CreditAttribution: helmo commented 7 June 2012 at 10:14

Status:

Needs work

» Needs review

For some reason upgrading my test box to Ubuntu 12.04 made all tests pass again :)

So I can confirm that they still pass after applying the patch from #5

Comment #12

sylus CreditAttribution: sylus commented 7 June 2012 at 16:56

Thanks helmo for your work on this :) Does this then mean we can bring this patch in? ^_^

Comment #13

jhedstrom

English

Portland, OR

CreditAttribution: jhedstrom commented 7 June 2012 at 17:04

I'd prefer to see this logic moved out of the actual curl call, perhaps by checking the response code?

Comment #14

helmo CreditAttribution: helmo commented 14 June 2012 at 08:29

@jhedstrom: do you mean that you would like --dump-header to save all headers and start parsing those ourselves?
That sounds like a complex/error prone solution.

In my opinion the --fail option is exactly what we need here.

From the manpage:

       -f, --fail
              (HTTP)  Fail  silently (no output at all) on server errors. This is mostly done to better enable scripts etc to better deal
              with failed attempts. In normal cases when a HTTP server fails to deliver a document, it returns an HTML  document  stating
              so (which often also describes why and more). This flag will prevent curl from outputting that and return error 22.

              This method is not fail-safe and there are occasions where non-successful response codes will slip through, especially when
              authentication is involved (response codes 401 and 407).