[GitHub] [ant] bodewig commented on pull request #126: support to access local properties via propertyset
bodewig commented on pull request #126: URL: https://github.com/apache/ant/pull/126#issuecomment-671948800 replaced with #135 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] [ant] bodewig commented on pull request #126: support to access local properties via propertyset
bodewig commented on pull request #126: URL: https://github.com/apache/ant/pull/126#issuecomment-667674899 @babasaikiran please take a look at #135 for my suggestion for fixing the bug This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] [ant] bodewig commented on pull request #126: support to access local properties via propertyset
bodewig commented on pull request #126: URL: https://github.com/apache/ant/pull/126#issuecomment-667667624 By returning `false` from `LocalPropertyStack`'s `setNew` you create a copy of the local property inside of the project properties themselves. I can see you do this because there is no way to enumerate all properties that are in scope - there are a few comments in `PropertyHelper` that indicate we have been aware of the problem back when we worked on Ant 1.8.0. What would probably be cleaner was to finally introduce a new `PropertyEnumerator` delegate along with new methods left and right and allow `PropertySet` to use that. I'll sketch this in a branch. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] [ant] bodewig commented on pull request #126: support to access local properties via propertyset
bodewig commented on pull request #126: URL: https://github.com/apache/ant/pull/126#issuecomment-666339391 It's been several years since I last looked at the implementation of local properties, something I need to change before I can merge your PR anyway. Right now I'm not sure of all the implications of your change to `LocalPropertyStack`. ``` stefan@numpad:~/devel/ASF/ant$ uname -a Linux numpad 5.4.0-29-generic #33-Ubuntu SMP Wed Apr 29 14:32:27 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux stefan@numpad:~/devel/ASF/ant$ java -fullversion openjdk full version "1.8.0_252-8u252-b09-1ubuntu1-b09" stefan@numpad:~/devel/ASF/ant$ git checkout -b babasaikiran-bug-50179 master Zu neuem Branch 'babasaikiran-bug-50179' gewechselt stefan@numpad:~/devel/ASF/ant$ git pull https://github.com/babasaikiran/ant.git bug-50179 Von https://github.com/babasaikiran/ant * branchbug-50179 -> FETCH_HEAD error: Terminal is dumb, but EDITOR unset Merge wurde nicht committet; benutzen Sie 'git commit', um den Merge abzuschließen. stefan@numpad:~/devel/ASF/ant$ ./bootstrap.sh ... Bootstrapping Ant Distribution ... BUILD SUCCESSFUL Total time: 6 seconds ... Cleaning Up Build Directories ... Done Bootstrapping Ant Distribution stefan@numpad:~/devel/ASF/ant$ ./build.sh antunit-tests -Dantunit.testcase=**/local-test.xml Buildfile: /home/stefan/devel/ASF/ant/build.xml ... antunit-tests: Created dir: /home/stefan/devel/ASF/ant/build/antunit/xml Build File: /home/stefan/devel/ASF/ant/src/tests/antunit/taskdefs/local-test.xml Tests run: 6, Failures: 1, Errors: 0, Time elapsed: 2,079 sec Target: testMacrodef took 0,005 sec Target: testSequential took 0,001 sec Target: testTarget took 0 sec Target: testGlobalLocal FAILED at line 28, column 21 Message: Assertion failed took 0,002 sec Target: testParallel took 2,019 sec Target: testBaseline took 0 sec BUILD SUCCESSFUL Total time: 8 seconds ``` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] [ant] bodewig commented on pull request #126: support to access local properties via propertyset
bodewig commented on pull request #126: URL: https://github.com/apache/ant/pull/126#issuecomment-665651176 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
