phingofficial / phing

PHing Is Not GNU make; it's a PHP project build system or build tool based on Apache Ant.
https://www.phing.info
GNU Lesser General Public License v3.0
1.16k stars 319 forks source link

"Late property expansion" and property arrays on 2.x? #616

Closed mpdude closed 3 years ago

mpdude commented 7 years ago

Hi all,

a long time ago, I have contributed two features I called "late property expansion" and "property arrays" on what was the "master" branch then; this later became (IIRC) the 3.0 branch and then unstable-3.0; maybe it is called "future-4.0" right now.

I then made another attempt (around early 2015?) and ported the changes to one of the newer "future" branches, the one that introduced namespaces as well.

Unfortunately, none of these branches has yet seen an official release, which is the reason why we're working with unstable/snapshot builds of Phing for years.

Both features are important for the way we're using Phing at my company, and I would make another attempt to get them merged because I'd be happy to work with a stable, mainstream Phing release.

So, what I would like to discuss here is under which conditions these changes would be accepted in the 2.x branch.

"Late property expansion" basically defers the expansion of ${property} expressions to the latest possible moment. For example, you could load a properties file like

webdir.name = web
paths.webdir = ${project.basedir}/${webdir.name}
paths.assets = ${paths.webdir}/assets

and then start Phing with -Dwebdir.name=www. This will end up with all properties using www as you might expect.

Of course, this brings a little risk of BC breaks for folks that happen to re-define Phing properties during buildfile execution but expect previous assignments to remain unchanged.

The other feature adds a little syntax to have "array" properties like so:

some.path[] = foo
some.path[] = bar

When inlining this property as ${some.path}, it will be glued together with ,. This makes it easier in some places to come up with generic tasks/buildfiles that can be configured by properties only – you cannot easily write the above with some.path.1 and some.path.2 properties, because there is (to my knowledge) no easy way of finding and iterating these properties.

What do you think? Is there a strict "no possibly breaking changes on 2.0" policy? I assume you would accept it if it could be turned on via command line switch?

mrook commented 7 years ago

@mpdude this is precisely the reason behind the new 3.0 branch, which I want to release early next year at the latest. 2.x should remain as is, but 3.0 can receive this patch. I'm willing to help wherever I can :)

mrook commented 7 years ago

@mpdude are you still interested in backporting this to 3.0 / master? I can assist where needed.

mpdude commented 7 years ago

Yes, definitely.

This feature is of big use for me and my team and the reason why we're stuck on the unstable-3.0 or even future-4.0 branch. To be honest, I lost track of where these changes actually live :-(

I was a bit hesitant during the last months to work on this because I have already implemented it twice on different branches that both got postponed.

So, where should the PRs be based – master or 3.0? What's the timeframe you'd need them?

mrook commented 7 years ago

@mpdude I understand. There were never any firm plans for those releases, real life got in the way mostly.

3.0 is master, so that'd be the branch to use. I'd like to release 3.0 somewhere in Q1 if feasible.

We probably need to centralize that discussion (on Slack for example).

stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

mpdude commented 6 years ago

Just for the record, I am still using this branch/feature in production every day and we’ve built a valuable, flexible build tool and process around it.

I understand that we cannot make these changes before a 3.0. It’s a bit of a catch-22 that I cannot invest the time to port these changes to yet another prospective 3.x branch unless a release seems realistic. On the other hand, I value (and probably most other users do as well) stability that much that I would not want to push a new major too much 🤷🏻‍♂️

If your intentions change or the time is ripe, let me know.

mrook commented 6 years ago

I'd say we can still do this in master; even though we have a first alpha out for 3.0, nothing is locked down yet. Are you able to cross-port this to current master?

mrook commented 6 years ago

@mpdude ^

mpdude commented 6 years ago

Basically yes, I‘d like to do this. Spare time is a bit of an issue right now... 😑

mrook commented 6 years ago

Understood! No rush :-)

kenguest commented 6 years ago

damn overeager bot...

On 21 August 2018 at 08:12, Michiel Rook notifications@github.com wrote:

Understood! No rush :-)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/phingofficial/phing/issues/616#issuecomment-414573380, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOShgDdmHrjG3crbAI1PD7lYlEXtrgvks5uS7LLgaJpZM4Km_Y7 .

-- http://about.me/kenguest/

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

mrook commented 3 years ago

I'm going to go ahead and close this for now. Phing has diverged pretty significantly on the property setting/expansion front since the commits referenced in this issue were authored. If/when a (new) need for this arises, we'll re-open.