You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by "rhoughton-pivot (GitHub)" <gi...@apache.org> on 2018/10/01 22:13:59 UTC
[GitHub] [geode] rhoughton-pivot opened pull request #2548: Better
path resolution for srcDist exclusions
Previous exclusion would count executable files named 'build' or 'out'
regardless of path, when we only want to exclude the build-tools output
directories.
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
- [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
- [ ] Is your initial contribution a single, squashed commit?
- [ ] Does `gradlew build` run cleanly?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
### Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.
[ Full content available at: https://github.com/apache/geode/pull/2548 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] pivotal-jbarrett closed pull request #2548: Better
path resolution for srcDist exclusions
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
[ pull request closed by pivotal-jbarrett ]
[ Full content available at: https://github.com/apache/geode/pull/2548 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] pivotal-jbarrett commented on pull request #2548:
Better path resolution for srcDist exclusions
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
This should be matching directories only anyway, so why the extra work to do it programmatically?
[ Full content available at: https://github.com/apache/geode/pull/2548 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] pivotal-jbarrett commented on pull request #2548:
Better path resolution for srcDist exclusions
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Well that’s just downright a big in Gradle then. Any open tickets on the issue?
[ Full content available at: https://github.com/apache/geode/pull/2548 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] rhoughton-pivot commented on pull request #2548:
Better path resolution for srcDist exclusions
Posted by "rhoughton-pivot (GitHub)" <gi...@apache.org>.
Because 'should be' isn't the same as 'is', and it matches on regular files named 'build' and 'out' also. `out` is a required file name of a concourse-resource, and it was being excluded by this line.
[ Full content available at: https://github.com/apache/geode/pull/2548 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] rhoughton-pivot commented on issue #2548: Better
path resolution for srcDist exclusions
Posted by "rhoughton-pivot (GitHub)" <gi...@apache.org>.
The file name cannot be changed.
On Mon, Oct 8, 2018, 08:34 Anthony Baker <no...@github.com> wrote:
> *@metatype* commented on this pull request.
> ------------------------------
>
> In geode-assembly/build.gradle
> <https://github.com/apache/geode/pull/2548#discussion_r223408364>:
>
> > @@ -320,8 +323,6 @@ distributions {
> exclude '**/gradlew.bat'
> exclude '**/gradle/wrapper/gradle-wrapper.jar'
> exclude '**/.gradle'
> - exclude '**/build/**'
>
> Eh, I would rather find a way to keep the build file simple. Instead of
> writing gradle code to work around this, how `bout changing the file
> names...?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/geode/pull/2548#discussion_r223408364>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/Aer30j4KXgJR-AW_cnbG_Yas8HTooAgXks5ui3CSgaJpZM4XC-6s>
> .
>
[ Full content available at: https://github.com/apache/geode/pull/2548 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] upthewaterspout commented on issue #2548: Better
path resolution for srcDist exclusions
Posted by "upthewaterspout (GitHub)" <gi...@apache.org>.
I'm fine with this change, but I do wonder if there is simpler way to do this in gradle - I think the exclude can take a closure, so maybe something like this would work?
```
exclude {it.path.contains('/out/') && !it.path.contains('/ci'}
```
https://docs.gradle.org/current/javadoc/org/gradle/api/file/CopySpec.html#exclude-org.gradle.api.specs.Spec-
[ Full content available at: https://github.com/apache/geode/pull/2548 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] metatype commented on pull request #2548: Better
path resolution for srcDist exclusions
Posted by "metatype (GitHub)" <gi...@apache.org>.
Eh, I would rather find a way to keep the build file simple. Instead of writing gradle code to work around this, how `bout changing the file names...?
[ Full content available at: https://github.com/apache/geode/pull/2548 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] rhoughton-pivot commented on issue #2548: Better
path resolution for srcDist exclusions
Posted by "rhoughton-pivot (GitHub)" <gi...@apache.org>.
@nabarunnag @upthewaterspout @metatype Please check these changes to our source distribution. This is a better answer than what we used for 1.7.0
[ Full content available at: https://github.com/apache/geode/pull/2548 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] pivotal-jbarrett commented on pull request #2548:
Better path resolution for srcDist exclusions
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
While I am not a fan of complicated builds I would rather keep the hacks for bugs closer to the source. In this case the bug is in Gradle so let's keep the fix in Gradle. Changing the Docker build to rename files to match the Concourse API would be making another build processes, removed from Gradle, responsible for Gradle's bug.
[ Full content available at: https://github.com/apache/geode/pull/2548 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org