zend-patterns / ZendServerSDK

Pure ZF2 CLI for zpk creation and webapi client.
BSD 3-Clause "New" or "Revised" License
22 stars 17 forks source link

Incorrect generation of scripts sub-directories #56

Closed ryanheath86 closed 9 years ago

ryanheath86 commented 9 years ago

Scripts directory within the ZPK was not being generated correctly for two reasons:

1) The files were contained within a double sub directory : /scripts/scripts 2) Sub-directories of the scripts folder were not being flattened, as is the case when packaging using Zend Studio

ryanheath86 commented 9 years ago

Apologies for adding two fixes in one, as you can see, I have fixed another bug which meant the icon file was not added to the top level directory of the package, rendering it invalid

mkherlakian commented 9 years ago

LGTM @slaff can you please review?

slaff commented 9 years ago

@ryanheath86 I've added your icon fix. For better accountability would be possible if you rebase your fork and resubmit?

boedah commented 9 years ago

How is this feature actually designed to work? There is no documentation on that (xml vs. properties):

After trying different combinations, we have in the xml:

<?xml version="1.0" encoding="UTF-8"?>
<package xmlns="http://www.zend.com/server/deployment-descriptor/1.0" version="2.0">
...
    <scriptsdir>scripts/zend-server-deployment</scriptsdir>
...
</package>

and in the properties:

# this must be directories
appdir.includes = \
    app,\
    src,\
    vendor
# this must be files
scriptsdir.includes = \
    scripts/zend-server-deployment/common.php,\
    scripts/zend-server-deployment/helper.php,\
    scripts/zend-server-deployment/post_stage.php,\
    scripts/zend-server-deployment/pre_activate.php,\
    scripts/zend-server-deployment/post_activate.php

And it works.

Will this break with this PR?

We also found out that appdir.includes must be directories while scriptsdir.includes cannot be directories.

Can this be documented better (and ultimately covered by #59)?

ryanheath86 commented 9 years ago

@boedah "We also found out that appdir.includes must be directories while scriptsdir.includes cannot be directories." - This was the problem I was having, packaging through Zend Studio allows directories to be included in scriptsdir.includes, the behaviour you see there is it copies all the files within that directory, along with all files in any sub-directories, into the scriptsdir defined in the XML - giving you a single directory in the package (named after scriptsdir) which contains all the files, flattened out with no sub-directories.

My update should match this behaviour, but I didnt check adding single files to the scriptsdir.includes as this looked to be handled fine already in the pack() method.

@slaff If you could give me an idea of how to "rebase and resubmit", I certainly can, but I'm still quite new to git! Can this be done in sourcetree, or can you give me some git commands?

ryanheath86 commented 9 years ago

@boedah @slaff BTW, i'm glad it's not just me that thinks these things are massively under-documented, if you guys aren't sure how it's supposed to work then I have no chance! :)

slaff commented 9 years ago

.. how to "rebase and resubmit"..

@ryanheath86: From your local copy of your fork do something like this: git pull --rebase https://github.com/zend-patterns/ZendServerSDK.git master This should put your changes on top. After that doing something like: git push -f Should push the changes to your remote repo . And finally create a new pull request.

ryanheath86 commented 9 years ago

@slaff That seemed to go horribly wrong, sorry! I don't understand why it keeps updating my original pull request instead of allowing me to open a new one. I may have ruined it by doing a fetch somewhere during the process. Anyway, I started again - here you go: https://github.com/zend-patterns/ZendServerSDK/pull/61

slaff commented 9 years ago

@boedah @ryanheath86 I am describing in small steps what is happening when Zend Studio is used to pack a ZPK( #60 ). That logic will be used as a guideline for a correct fix.