Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bwood’s picture

It doesn't look like it. I turn to the code when I don't find info in the docs. If you look at drush_make.project.inc you can see that only option for retrieving patches is wget. Grabbing patches from svn is something that I'd like too. Since there is a means for getting projects from svn, it might not be hard to add this for patches. Maybe you or I will find time to roll a patch for this new feature at some point.

christopheperrin’s picture

Version: 6.x-2.0-beta6 » 6.x-2.0-beta9
Status: Active » Needs review
FileSize
2.64 KB

Hi.

I created a drush make patch for downloading patches from a SVN server.
This patch has been created for version 6.x-2.0-beta9 (there has been a lot of refactoring in beta9 and the patch won't work with earlier version of drush make) so make sure you use the right version.

you need to add the following to your makefile

projects[myProject][patch][download][type] = "svn"
projects[myProject][patch][download][url] = "http:///
"
projects[myProject][patch][download][filename][] = "patch1.patch"
projects[myProject][patch][download][filename][] = "patch2.patch"
projects[myProject][patch][download][username] = ""
projects[myProject][patch][download][password] = "
"

Let me know if you have any issues using the patch

univate’s picture

I would also like to be able to apply patches from a local directory and not just a url, that way I could store patches (for site specific things) along side my make file.

dmitrig01’s picture

Status: Needs review » Active

This is a good idea but not the approach I want to take - see #922026: Make it possible to download single files that do not get uncompressed and we'll need some code here.

eft’s picture

dmitrig01 - I couldn't understand the approach you want to take from #922026: Make it possible to download single files that do not get uncompressed. In that issue on the same date you noted that the issue of not uncompressing single files had been addressed. So, I am confused. Can you clarify the approach you would like adopted for the issue in this thread i.e. applying patches from a source other than an URL.

Thanks

dmitrig01’s picture

Status: Active » Postponed

I want to use drush_make_download_factory + #919224: Allow use of subtrees or individual file from archives instead of the whole archive, so you can pull a patch in from any source. I'm going to mark this postponed until that issue is resolved.

izmeez’s picture

subscribing. I am searching now to find any tips on how to use a local patch file with drush make.

noels’s picture

Patching from local files was very important for me, so I've created a patch against the current version of drush_make. It works for me. YMMV

To use a local file, you will want to use a url starting with: file://

izmeez’s picture

@noels I'm having some difficulties with the patch in #8,

The patch applies without any problems.

I am then testing it with a make file as follows:

; Core version and api
; --------------------

core = 6.x
api = 2

projects[drupal][version] = "6.20"

; private download patch, http://drupal.org/node/572516
; also need to mkdir misc/dynamic
; projects[drupal][patch][] = "http://drupal.org/files/issues/drupal-6.20-572516_67.patch"
 
; Patch using local files
projects[drupal][patch][url] = "file://var/aegir/Downloads/d6patches/core/572516_67-drupal-6.20-private-download.patch"
; also need to mkdir misc/dynamic

Regardless of setting file ownership and permissions,
and regardless of using [patch][] = or [patch][url] =

the following error occurs
sh: cannot open var/aegir/Downloads/d6patches/core/572516_67-drupal-6.20-private-download.patch: No such file

and there is no PATCHES.txt file created by drush make.

I'm not sure what's wrong. Any thoughts?

Yes, the local file name is correct and different from the one on drupal.org

Thanks,

Izzy

noels’s picture

Hi Izzy

The file you have specified is relative to your current directory. try file:///var/aegir/Downloads/d6patches/core/572516_67-drupal-6.20-private-download.patch

Noel

izmeez’s picture

@noels, Thanks. Yes, the extra "/" was the answer.

However, the original patch file is deleted in the process.

Izzy

mxmilkiib’s picture

re locally stored files; i was unsure if this was possible, and while i saw this thread, it seemed just patch related so i posted #1157272: Syntax for locally stored file?, but univate hints there that the patch above could be expanded for non-patch files, so i thought i'd ask if that might be possible? or should i turn the other thread into a separate feature request? thanks.

izmeez’s picture

Does anyone have any suggestions as to why the original patch files get deleted as described in comment #11 ?
Thanks.

girishmuraly’s picture

@izmeez, the reason for deletion is due to the call drush_op('unlink', $filename);

Attached is the fix for that reapplied to patch from #8. It is untested though.

izmeez’s picture

@girishmuraly Thanks. I see the patch in #14 is quite different from the patch in #8.

The patch in #14 fails to apply hunk#1 to drush_make version 2.2 so I applied it manually.
The patch appears to work and does not delete the original patch files.

However, I noticed it did not create a patches.txt file to show what patches were applied with drush_make. On close examine I see two of my test patches even though they applied properly gave rise to the creation of .orig files possibly because the hunk offset was different. There are no .rej files. I would have thought in this case the patches would have been determined to have been applied and the patches.txt file created. I am not sure if this is a new issue or related to this patch to drush_make.

Izzy

izmeez’s picture

Version: 6.x-2.0-beta9 » 6.x-2.2
Status: Postponed » Needs review
FileSize
1.85 KB

Attached is a new patch file against drush_make 6.x-2.2 that combines the patches from #8 and #14. This supports local patch files and preserves the original patch files. I have changed the status to needs review.

izmeez’s picture

My apologies for posting this twice but I think I should have renamed the patch file more appropriately. I am unable to delete the first file so I am attaching the same file with a better name.

j0nathan’s picture

Subscribing, to be able to apply patches from local directories.

likewhoa’s picture

sub this interest me

paul.lovvik’s picture

The patch no longer applies. Rerolling it.

I have put the code that applies a patch into its own function so it can be leveraged for this purpose. This change allows your local patch to have a strip number of either 0 or 1.

izmeez’s picture

Status: Needs review » Reviewed & tested by the community

The patch in #21 applies to 6.x-2.x-dev and is working with local patch files.

Can't comment on use with 6.x-2.2

izmeez’s picture

Version: 6.x-2.2 » 6.x-2.3

Confirming that patch in #21 applies to drush_make-6.x-2.3 and works as expected.
Is commit possible?

j0nathan’s picture

Status: Reviewed & tested by the community » Needs work

Hi,
Thanks for the work you are doing.

Related to patch in #21 with drush make 2.3:

As I understand, the file is called with an absolute path or a relative path, but relative from where the drush make command is executed.

In our case, it would be better to call the file with a path relative to the makefile itself where the file is called.

Our makefiles are into a git repository which contains different sub-directories with patches. So the patch should be called with a path relative to the makefile. Like this, I would be able to go into any directory on the system where I want to create a platform and execute "drush make /path/to/git/makefiles/stub-makefile.make". This example would apply the patch "../patches/mypatch.patch" instead of having to hard code the absolute path "/path/to/git/patches/mypatch.patch" in the makefile. This is also because we don't know where in the system the git directory would be.

What do you thing about that?

Steven Jones’s picture

subscribe.

skwashd’s picture

Version: 6.x-2.3 » 6.x-2.x-dev
Status: Needs work » Needs review
FileSize
9.21 KB

The attached patch implements support for file:/// URLs in a more generic way. I fixed some broken tests while implementing this change.

Edit: I think this issue should be split into 2 parts, one for file:/// support and the other for using partial trees.

helmo’s picture

Status: Needs review » Closed (won't fix)

[ Powered by #1115636: Issue Macros and Templates - Drush Make]

Drush make is being merged into drush core (discussed in issue:#1310130: Put drush make in drush core)
This means that the issue queue is also moving. The Drush project has a component called 'Make' for this purpose.

We would like to take this opportunity to leave behind old/obsolete issues, allowing us to focus on a stable make command in core. E.g. one of the major tasks ahead is making more use of the Drush core code for handling downloads and decompression.

If you feel that this issue is still relevant, feel free to re-open and move it to the Drush queue.

More information will be posted on the drush_make and drush project pages.

smokris’s picture

Title: Is it possible to specify a patch from SVN or local file system? » Specify a patch from SVN or local file system
Project: Drush Make » Drush
Version: 6.x-2.x-dev »
Component: Miscellaneous » Make
Category: support » feature
Status: Closed (won't fix) » Needs work

This issue is still relevant; we need a way to apply patches from a Subversion repository.

Steven Jones’s picture

Title: Specify a patch from SVN or local file system » Allow specifying a patch from the local file system

Moving to the drush project, and removing reference to SVN, as that never seemed to get traction in this issue.

I also agree with https://drupal.org/node/840540#comment-5010426, that we shouldn't commit this unless we can specify files relative to the make file.

Steven Jones’s picture

Title: Allow specifying a patch from the local file system » Specify a patch from SVN or local file system

Apparently SVN is still needed for some.

helmo’s picture

mstef’s picture

Local patches apply fine for me *only* if they are patching core. Local patches are failing if they're for contrib modules, even though the entries in the make file are like: projects[media][patch][] = "file://../patches/media-default_formatter.patch"

Must have to do with the git apply --check command used to see if the patch is clean.

helmo’s picture

@milkmiruku: please note that make first does a chdir to /tmp, so in your case "/patches/media-default_formatter.patch" would be looked up.

This might be a problem if the patch resides in the same repository as the makefile though....

mstef’s picture

Well, that sort of kills the local functionality in my mind, if you can't do something like file://../patches/. It means the make file cannot be portable at all - and that's awful. Is there anyway at all we can reference the root where the make file resides?

EDIT: Odd, because it seems to work fine when I'm patching core with a local patch (file://../patches/something.patch), but does not work when trying to patch contrib.

igor.ro’s picture

Hello

I extend patch from comment #26 to allow use patches related to make file.
as I know "file://../patches/media-default_formatter.patch" is not correct for specification, that will think about .. as a host and
path will be /patches/media-default_fortmatter.patch. You can test this with parse_url function.

That is why I suggest to use simple path like

projects[pressflow][type] = "core"
projects[pressflow][download][type] = "file"
projects[pressflow][download][url] = "http://launchpad.net/pressflow/6.x/6.22.104/+download/pressflow-6.22.104..."
projects[pressflow][patch][fork-fix] = "patches/core/system.fork-fix.patch"

Works fine for me.
Tested:
Ubuntu 11.10
PHP 5.3.6-13ubuntu3.3 with Suhosin-Patch (cli)
Drush 5.x

mstef’s picture

(edit out)

greg.1.anderson’s picture

Status: Needs work » Needs review

+1 for projects[modulename][patch][] = "patches/path/modulepatch.patch"

I haven't had time to test #35 yet, but the code looks reasonable. I wonder if the call to realpath in preprocessLocalFileUrl couldn't simply be done via realpath($this->make_directory . '/' . $uri['path']). I suppose you'd have to wrap that in if (drush_is_absolute_path($uri['path'])) in order to get the right result when the path is absolute. That seems cleaner to me, but I suppose the chdir works too. I just have an instinct against changing the cwd unless it's necessary. Whatever technique we use, perhaps we should make a wrapper for this (drush_relative_realpath?) in includes/filesystem.inc so that we do it consistently wherever it is needed.

This next comment is about Drush make, not the patch in #35, but I cannot help but comment on drush_shell_exec("ls %s", $download_path);, which really should be done with drush_scan_directory. If this isn't fixed in this issue, perhaps someone could make another issue to track it. It's minor, but easy to do and worthy of attention.

igor.ro’s picture

greg.1.anderson

You are right about realpath($this->make_directory . '/' . $uri['path'])
that would be a prefect way, I just did not know about function drush_is_absolute_path($uri['path']).
Looks like you are drush ninja :) I'll redo patch.

Thanks.

igor.ro’s picture

Fix with greg.1.anderson suggestions #37.

Thanks

greg.1.anderson’s picture

Okay, looking much better, but I had another thought. Once you have the full path to the actual file with realpath, wouldn't it be better to just gen up a "file:///" URI and dump _drush_make_request_local_file? That's kind of an odd function.

igor.ro’s picture

Greg, probably I do not get your idea well, but this is my thought.

  protected function preprocessLocalFileUrl($url) {
    $uri = parse_url($url);
    if (!isset($uri['scheme']) && isset($uri['path'])) {
      $path = $uri['path'];
      if (!drush_is_absolute_path($uri['path'])) {
        $path = $this->make_directory .'/'. $path;
      }
      $path = realpath($path);
      $url = "file://$path";
    }
    return $url;
  }

allow us to make absolute or relative path to uri file:///{absolute_path}.
Then we pass that uri common way to there download function. In that case we make minimum changes to get it work.
Calling preprocessLocalFileUrl() method in each point we going to start download, allow us to use local pathes for libraries, translations and projects too. With patch like http://drupal.org/node/919224 we will get powerfull tool.

Happy New Year, btw.

greg.1.anderson’s picture

Yes, #41 is what I meant exactly.

igor.ro’s picture

I just copied code from #39 to #41.
So our minds are the same.

greg.1.anderson’s picture

Does that mean that the call to _drush_make_request_local_file can be removed in make_http_request, then? That would be a good improvement.

igor.ro’s picture

I think we can not remove _drush_make_request_local_file, because it incapsulate local request as http request interface.
That code will be used in case we do not curl installed.
Also it handle case when user define absolute path with file:///
I do not think this improvement is critical.

greg.1.anderson’s picture

Assigned: Unassigned » jhedstrom
Status: Needs review » Reviewed & tested by the community

I see. make_http_request is only called from _make_download_file, which ultimately will go away entirely. Since this is an interim solution, I guess it is fine as is. Otherwise, I would point out that fopen accepts file:/// scheme URIs in place of $filename; if this code was going to stick around, I think it could be cleaned up along those lines, which would simplify it significantly. Also, drush presently requires wget or curl for the pm-* routines, so I don't know that it's worth maintaining curl replacements in Drush make.

jhedstrom’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

I'd like to see a test added for this functionality before committing it. It should be relatively straight-forward to add a local patch in the tests/makefiles directory.

Garrett Albright’s picture

Status: Needs work » Needs review
FileSize
9.63 KB

Here's my go at it. It's #39 with two main improvements; first, it doesn't cause "missing parameter" warnings when recursive .make files are used, and second, it adds a test. However, though I can get things to work just fine if I use it "normally," the patch doesn't "download" when I run the test. I think this is because "download-mechanism" is being force-set to "curl" somehow when the test is run (it doesn't seem to be set at all under normal operation). As greg says in #46, this will likely be a non-issue if/when _make_download_file() goes away

greg.1.anderson’s picture

Looks fine.

Minor point: since the 'file:' is not required, could the test be projects[wysiwyg][patch][] = "patches-local-test-wysiwyg.patch" instead of projects[wysiwyg][patch][] = "file:patches-local-test-wysiwyg.patch"?

Garrett Albright’s picture

I don't like that idea, since projects[wysiwyg][patch][] is shorthand for projects[wysiwyg][patch][foo][url], and if it doesn't have the `file:` schema, it's not really a URL. In fact, maybe that's a bug that it works without it…

greg.1.anderson’s picture

Relative URLs are valid URLs. ex: <img src="images/picture.png">. We are just defining in drush make that relative URLs are relative to the makefile.

jhedstrom’s picture

Status: Needs review » Needs work

These tests aren't passing for me locally--since the md5 sums in tests/makeTest.php haven't been updated to match the changes to patches.make, I'm guessing they aren't passing for others?

Garrett Albright’s picture

Okay, looking into this further, it seems there are deeper problems. It seeems to actually be using curl to "download" files using the files: protocol, which works in certain fluke cases but usually don't. Maybe this whole thing should be put aside until Make's download functions are eradicated for good, and then we can make sure that Drush has support for "downloading" local files. This feature would be really useful, though, so I hope that comes soon.

igor.ro’s picture

can tThis issue be closed?
I have just tested
drush 5 apply local patches fine. (did not test recursion)

jhedstrom’s picture

Status: Needs work » Fixed

This is working fine as far as I can tell. Local patches can be specified by absolute path:

projects[foo][patch][] = "file:///path/to/file/patch.txt"

or by a relative path

projects[foo][patch][] = "patch.txt"

where patch.txt resides in the same directory as the make file.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

smokris’s picture

Title: Specify a patch from SVN or local file system » Specify a patch from local file system

Changing title to reflect the scope of this issue.

kotnik’s picture

Just a note if somebody stumbles upon this issue later.

If you set:

projects[foo][patch][] = "patches/patch.txt"

First, Drush will attempt to download that URL. Which means that if you or your company redirects non existing URL to some landing page, you will get some content and applying patch will fail always.

Locally, I solved this by adding 'patches' string to the hosts file pointing to 127.0.0.1.

timfernihough’s picture

Thanks @kotnik! I did indeed have this problem and had come to another conclusion of using file:// but that appears to only work in some environments. I'll give your suggestion a shot.

torotil’s picture

I just tried the relative path like in #55 and it didn't work. I'm using drush 5.7 - reopen?

torotil’s picture

Status: Closed (fixed) » Needs work
igor.ro’s picture

yes relative pathes works with patche #41
We did not include it to the module because from system point of view it is not correct.

torotil’s picture

alright. what would be the correct approach to it? some extra protocol handlers like: makefile://relative/path/to/patch or cwd://relative/path ?

igor.ro’s picture

For me this is good idea.
local://relative/path/to/patch

igor.ro’s picture

For me this is good idea.
local://relative/path/to/patch

torotil’s picture

concerning local:// : isn't that exactly what file:// is for? maybe we should simply stick with that?

torotil’s picture

FileSize
6.78 KB

Ok. You're right file:// is not meant to handle relative paths. Here is a port of Garett's patch from #53.

It implements a new protocol local:// that handles local paths. In addition to absolute paths it also handles paths relative to the makefile.

torotil’s picture

Status: Needs work » Needs review
kotnik’s picture

Status: Needs review » Needs work

Hi torotil.

Patch seems okay, but please remove trailing whitespaces.

torotil’s picture

Status: Needs work » Needs review
FileSize
6.78 KB

Sorry, I have had my editor misconfigured (=configured for python). Here's the patch without trailing whitespace.

greg.1.anderson’s picture

Status: Needs review » Needs work

Is this latest patch a reaction to #58? I think that issue should be fixed by skipping the download steps (wget, curl et.al.) if the URL does not contain a protocol.

Local patches should be specified per #55. I am not in favor of "local://"; it is nonstandard and unnecessary.

jhedstrom’s picture

It would be nice to tackle #1569862: Drush Make local patch file is cached at the same time.

torotil’s picture

Status: Needs work » Needs review
FileSize
7.12 KB

Here's a patch that deals with relative paths in makefiles as well as with local (=non-protocol) paths.

* relative/path/to/file : is turned into an absolute path using the makefile's directory
* /absolute/path/to/file : will simply be copied (without cache)
* file:///absolute/path/to/file : works exactly as without this patch (with caching)

greg.1.anderson’s picture

Status: Needs review » Needs work

Thank you for #73; the code looks pretty good. However, I think that file://absolute should behave the same as /absolute, and not cache. There isn't a good reason for these to behave differently, nor is there a good reason to cache a local file, so consistency is preferable.

torotil’s picture

Status: Needs work » Needs review
FileSize
7.98 KB

file:/// urls in make-files aren't cached either now …

torotil’s picture

Is there anything holding this up?

fgm’s picture

This is also related with #1837808: Include a "directory" download method to "download" from local directories, which enables such local downloads not only for patches.

dkingofpa’s picture

Version: » 7.x-5.8

It would also be nice to patch drush's documentation so people know this functionality is available and how to use it.

moshe weitzman’s picture

Version: 7.x-5.8 » 8.x-6.x-dev
igor.ro’s picture

does this #75 patch merged into dev branch?

torotil’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
9.29 KB

Here's a reroll of my patch that adds a few lines to docs/make.txt that explain how to use local files.

Currently the patch also cleanly applies to 8.x-6.x!

Let's finally have this in drush. I'm setting this to RTBC as this has been reviewed a few times already.

torotil’s picture

The patch from #1837808: Include a "directory" download method to "download" from local directories is rather complementary than a substitute for this one.

igor.ro’s picture

#81 Works fine
please merge into 8.x-6.x-dev brach
thanks

jhedstrom’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: -Needs tests

I'm seeing a test failure specifically related to this patch:

1) makeMakefileCase::testMakePatch
Test patching and writing of PATCHES.txt file - build md5 matches expected value: 8dcdfd34df1c9e3490ffd4d65e11f600
Failed asserting that ' >> Project context contains 3 modules: context_ui, context_layouts, context.
 >> http://drupal.org/files/issues/custom_blocks_arent_editable-make.patch
 >> http://drupal.org/files/issues/661094-context-permissions.patch
 >> http://drupal.org/files/issues/features-drush-backend-invoke-25.patch
 >> http://drupal.org/files/0001-feature.inc-from-624018-211.patch
 >> patches-local-test-wysiwyg.patch
7c6bd31d3435f289a92e7aa7730ae3ea' contains "8dcdfd34df1c9e3490ffd4d65e11f600".
torotil’s picture

Status: Needs work » Needs review
Issue tags: +Needs tests
FileSize
9.29 KB

Adjusted checksum.

jhedstrom’s picture

Status: Needs review » Fixed

Committed #85 in 0a8b712. Thanks all!

igor.ro’s picture

Did we decline idea about special schema for local patches (local:///)?

greg.1.anderson’s picture

Status: Fixed » Needs work

This breaks the make tests:

There were 2 failures:

1) makeMakefileCase::testMakePatch
Test patching and writing of PATCHES.txt file - build md5 matches expected value: 7c6bd31d3435f289a92e7aa7730ae3ea
Failed asserting that <text> contains "7c6bd31d3435f289a92e7aa7730ae3ea".

/home/ga/local/drupal/drush/tests/makeTest.php:41
/home/ga/local/drupal/drush/tests/makeTest.php:62

2) makeMakefileCase::testMakeSubtree
Unexpected exit code: /home/ga/local/drupal/drush/drush --nocolor make --no-core /home/ga/local/drupal/drush/tests/makefiles/subtree.make /tmp/drush-sandbox/subtree
Failed asserting that <integer:1> matches expected <integer:0>.

/home/ga/local/drupal/drush/tests/drush_testcase.inc:349
/home/ga/local/drupal/drush/tests/drush_testcase.inc:416
/home/ga/local/drupal/drush/tests/makeTest.php:220

FAILURES!
Tests: 27, Assertions: 83, Failures: 2, Skipped: 1.

Probably just false negatives that need to be brought up to date with the latest code.

jhedstrom’s picture

Hmm, I'm seeing the tests pass locally (php 5.4 and 5.3 via virtualbox), but will look into this more.

joestewart’s picture

I don't think an md5 test will work for this since the PATCHES.txt contains the full path to the local patch.

jhedstrom’s picture

@joestewart good catch!

jhedstrom’s picture

It seems like the proper way to fix this is to copy any local patches into the relevant directories where PATCHES.txt is being written out, and have that file use a relevant path. Otherwise, a PATCHES.txt file with paths to unavailable patches doesn't seem very useful.

jhedstrom’s picture

Status: Needs work » Needs review
FileSize
4.04 KB

This patch keeps the md5 test, but resolves the issue by writing out a *relative* path for local patches, and moving those patches into the project directory with the PATCHES.txt file.

jhedstrom’s picture

Issue tags: -Needs tests

Removing the 'needs tests' tag.

jhedstrom’s picture

Status: Needs review » Fixed

I've committed #93.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

hanoii’s picture

Issue summary: View changes

Old issue, but I do have a question and worth adding it here.

If using a local patch, what's the proper approach, having it on a separate dir within the project or directly in the module dir? I am doing the latter otherwise I'll end up with both patches under the git project of my site, and this seems to work, but wonder what's the proper/recommended way of doing it.

jhedstrom’s picture

Re #97, I've typically added them in a 'patches' directory relative to the .make file. You will end up with 2 copies if you are committing the built site, but this way it is unlikely somebody who works on the site will miss the patch (in the case where they don't use or are unaware of drush make).