It would be nice if the project contained a drush makefile that downloads the library to the right spot. This means it can be used on services such as simplytest.me.

CommentFileSizeAuthor
#6 2215991-makefile-6.patch460 bytescafuego
#5 2215991-makefile-5.patch557 bytescafuego
#4 2215991-makefile-4.patch263 bytescafuego
#1 2215991-makefile.patch548 bytescafuego
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cafuego’s picture

cafuego CreditAttribution: cafuego commented
Status: Active » Needs review
FileSize
2215991-makefile.patch548 bytes

Attached makefile should do the trick for the D7 version.

Konstantin Komelin’s picture

Konstantin Komelin CreditAttribution: Konstantin Komelin commented

Thank you @cafuego. Great idea!
I'll review your patch soon.

Konstantin Komelin’s picture

Konstantin Komelin CreditAttribution: Konstantin Komelin commented
Status: Needs review » Postponed (maintainer needs more info)

Hi,

I'm trying to run your make file.

First of all, we need the following parameter:
api = 2

The second my concern is related to the type of download, it should be:
libraries[simpelhtmldom][download][type] = "file"
instead of
libraries[simpelhtmldom][download][type] = "get"

Finally, I would find constant direct link instead of time-dependent one:
libraries[simplehtmldom][download][url] = "http://downloads.sourceforge.net/project/simplehtmldom/simplehtmldom/1.5/simplehtmldom_1_5.zip?r=&ts=1394605241&use_mirror=internode"
For example. we can try to use:

libraries[simpelhtmldom][download][type] = "svn"
libraries[simplehtmldom][download][url] = "http://svn.code.sf.net/p/simplehtmldom/code/trunk"

All this comments are based on drush make documentation http://www.drush.org/docs/make.txt

I have a question to you. Have you tried to execute your make file?

Thanks,
Konstantin

cafuego’s picture

cafuego CreditAttribution: cafuego commented
Status: Postponed (maintainer needs more info) » Active
FileSize
2215991-makefile-4.patch263 bytes

I didn't test-run the makefile, just wanted to get something in here before I forgot to do it :-)

The download type of GET is correct - at least for the drush version I've got. It just means drush should do an HTTP GET request.

I've looked for a fixed zip file download link, but I can't find a sensible one. I considered an SVN checkout, but I'm not convinced that grabbing a non-numbered and possibly unstable version is a good idea. Having said that, I don't know if the library is even still maintained..

Attaching an updated patch and testing that via simplytest.me.

cafuego’s picture

cafuego CreditAttribution: cafuego commented
FileSize
2215991-makefile-5.patch557 bytes

Tssk, clearly need more coffee. Re-created patch properly.

cafuego’s picture

cafuego CreditAttribution: cafuego commented
Status: Active » Needs review
FileSize
2215991-makefile-6.patch460 bytes

And attached is a patch with a working makefile, containing a direct zip file link. It really helps if I don't typo the array key name for the download type field.

It seems to work if I try it with one of my modules.

Konstantin Komelin’s picture

Konstantin Komelin CreditAttribution: Konstantin Komelin commented
Status: Needs review » Fixed

Thank you @cafuego for your contribution!

I've committed your patch: http://drupalcode.org/project/simplehtmldom.git/commit/55ab98f
The only change I made according to documentation was:
libraries[simpelhtmldom][download][type] = "file"

You may take a look at Drush Make Generator output:
http://drushmake.me/file.php?token=bc61ffba95b0

Also, I've added a link to your BOM Weather Lite module to the simplehtmldom project page to promote it a little bit if you don't mind :)

Thanks,
Konstantin

Konstantin Komelin’s picture

Konstantin Komelin CreditAttribution: Konstantin Komelin commented

Also, committed to 6.x-2.x branch: http://drupalcode.org/project/simplehtmldom.git/commit/af518bd

Thanks again.

cafuego’s picture

cafuego CreditAttribution: cafuego commented

I don't mind at all - thank you konstantin :-)

  • Commit 55ab98f on 7.x-2.x, 8.x-2.x authored by cafuego, committed by konstantin.komelin: 
    Issue #2215991 by cafuego, konstantin.komelin: Provided makefile that...

Status: Fixed » Closed (fixed)

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

mvonfrie’s picture

mvonfrie CreditAttribution: mvonfrie commented

Linking the library as file and not as SVN repository is ok, but linking to this specific file is a bad idea. The version in this file uses a highlz outdated SVN repository format which can bring you in big trouble if your drupal profile is versioned with (latest) SVN itself and not with git. Shortly, it crashes your whole local SVN and you need to fix this which costs a lot of time.

I see two options:
* Extract the linked file, remove all SVN metadata (the hidden .svn folders recursively), zip it again and upload it to a place und your control, maybe to this projects git.
* Link the library with SVN to https://svn.code.sf.net/p/simplehtmldom/code/tags/v_1_11 (which currently is the latest release). I would not link to trunk because that's similar to dev branches of drupal modules.

mvonfrie’s picture

mvonfrie CreditAttribution: mvonfrie commented
Status: Closed (fixed) » Needs work
Konstantin Komelin’s picture

Konstantin Komelin CreditAttribution: Konstantin Komelin commented
Status: Needs work » Closed (fixed)

Hi @mvonfrie,

Thank you for your request.
However, I think the closed issue is not an appropriate place for posting it.

In this issue we've implemented standard drush make file that doesn't allow as to change content of the archive.
Please post to Drush project https://www.drupal.org/project/drush if you need the .svn filter feature to be implemented.

The 1.11 version of simplehtmldom library is outdated that's why we upgraded to 1.5 which is stable and recommended release http://sourceforge.net/projects/simplehtmldom/files/.
The simplehtmldom module doesn't contain any library at all. Feel free to download and install any version you like.

Moreover, I didn't find any .svn folders in the archive, so there is nothing to do here.

If you still see the issue please create a separate issue for your problem because we've finished this one.

Thanks,
Konstantin