[GitHub] ant-ivy issue #73: Enable XML report parser to produce qualified extra attri...
Github user twogee commented on the issue: https://github.com/apache/ant-ivy/pull/73 Sorry, my reasoning took the wrong turn, must be the heat ð¦Yes, passing qualified attributes to XmlReportWriter is the way to go. I wonder what was the reason for using "extra-" hack? I assume it was conceived as a way to avoid dealing with with random namespaces in the XML report. If it makes sense to continue with that, a way to encode the namespace prefix must be figured out. That means one should look at the permitted characters in the [XML Specification](https://www.w3.org/TR/xml11/#NT-NameChar) and try to figure out a separator such that from a name constructed as "extra-" a namespace prefix and a name could be extracted unambiguously. I propose using something like ".-_-." as a separator since it is unlikely to occur in any namespace prefixes for extra attributes describing Ivy artifacts (if somebody does that, it must be on purpose and thus an attempt to shoot oneself in the foot). Or should we rather deal with namespaces in XML reports? P.S. You're right about the Jira issue, please add it to the name of the PR (and eventually to the commit name). --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant-ivy issue #73: Enable XML report parser to produce qualified extra attri...
Github user aprelev commented on the issue: https://github.com/apache/ant-ivy/pull/73 By the way, found [issue IVY-1104](https://issues.apache.org/jira/browse/IVY-1104) in your issue tracker, seems to be the case. --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant-ivy issue #73: Enable XML report parser to produce qualified extra attri...
Github user aprelev commented on the issue:
https://github.com/apache/ant-ivy/pull/73
As of now, two sets of dependencies are constructed in a different ways:
- *currently resolved dependecies* are produced by Ivy resolve engine, with
**both qualified and unqualified attributes** (`super.qualifiedExtraAttributes
!= super.extraAttributes`), and
- *previously resolved dependencies* are parsed from last report by
`XmlReportParser`, with **unqualified attributes only**
(`super.qualifiedExtraAttributes == super.extraAttributes`).
Set comparison at `ConfigurationResolveReport::checkIfChanged()` invokes
`ModuleRevisionId::equals()` for individual dependencies comparison, which
compares `super.qualifiedExtraAttributes` maps as in:
```Java
@Override
public boolean equals(Object obj) {
...
return other.getRevision().equals(getRevision())
&& !(other.getBranch() == null && getBranch() != null)
&& !(other.getBranch() != null &&
!other.getBranch().equals(getBranch()))
&& other.getModuleId().equals(getModuleId())
&&
other.getQualifiedExtraAttributes().equals(getQualifiedExtraAttributes()); //<
here
}
```
which, obviously, yields a `false`, since keys in
`other.getQualifiedExtraAttributes()` map are stripped of qualifiers as
explained above.
That is why I introduced parameterised version of
`ExtendableItemHelper::getExtraAttributes()` used by `XmlReportParser`, this
way parsed dependencies have both versions of qualifiers, same as resolved
dependencies.
As I pointed out in original PR message, issue may instead be solved by
modifying `ModuleRevisionId::equals()` to use `super.extraAttributes`, provided
attributes namespaces cannot clash, of course, which will result in ignoring
qualifiers:
```Java
@Override
public boolean equals(Object obj) {
...
return other.getRevision().equals(getRevision())
&& !(other.getBranch() == null && getBranch() != null)
&& !(other.getBranch() != null &&
!other.getBranch().equals(getBranch()))
&& other.getModuleId().equals(getModuleId())
&& other.getExtraAttributes().equals(getExtraAttributes()); //<
ignoring qualifiers
}
```
At what point do you suggest we check for presense of `':'` in the names of
attributes?
---
-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant-ivy issue #73: Enable XML report parser to produce qualified extra attri...
Github user twogee commented on the issue: https://github.com/apache/ant-ivy/pull/73 Gradle seemed to have [different ideas](https://github.com/gradle/gradle/issues/954) about Ivy XML namespaces (not to be confused with Ivy [namespaces](http://ant.apache.org/ivy/history/2.5.0-rc1/settings/namespaces.html) ð); perhaps we should keep the parser namespace-unaware and simply check for presence of `':'` in the attribute names of the artifact descriptors? --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant-ivy issue #73: Enable XML report parser to produce qualified extra attri...
Github user aprelev commented on the issue: https://github.com/apache/ant-ivy/pull/73 @twogee Can you point me in the right direction regarding namespace URL? And what are your thoughts on including the namespace prefix in the report? --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant-ivy issue #73: Enable XML report parser to produce qualified extra attri...
Github user twogee commented on the issue: https://github.com/apache/ant-ivy/pull/73 @aprelev the latter, you can push a new commit. What I meant is that hardcoding "e:" is probably a bad idea, one may choose a different namespace prefix; rather, we should be enforcing use of namespace URL for extra attributes. --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant-ivy issue #73: Enable XML report parser to produce qualified extra attri...
Github user jaikiran commented on the issue: https://github.com/apache/ant-ivy/pull/73 >> Should I reopen PR? --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant-ivy issue #73: Enable XML report parser to produce qualified extra attri...
Github user aprelev commented on the issue: https://github.com/apache/ant-ivy/pull/73 Should I reopen PR? --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant-ivy issue #73: Enable XML report parser to produce qualified extra attri...
Github user jaikiran commented on the issue: https://github.com/apache/ant-ivy/pull/73 retest this please --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant-ivy issue #73: Enable XML report parser to produce qualified extra attri...
Github user aprelev commented on the issue:
https://github.com/apache/ant-ivy/pull/73
> Rather than hardcoding prefix, shouldn't we retrieve it by URL
"http://ant.apache.org/ivy/extra";?
Can you elaborate on prefix retrieving? The **extra-** prefix is [hardcoded
at](https://github.com/apache/ant-ivy/blob/5918182e0d6836d89c42260da9de4428d4cbcec0/src/java/org/apache/ivy/plugins/report/XmlReportWriter.java#L175)
`XmlReportWriter::extraToString()` regardless of attribute namespace:
```Java
sb.append(prefix).append("extra-").append(entry.getKey()).append("=\"")
.append(XMLHelper.escape(entry.getValue())).append("\"");
```
Maybe including namespace in XML report by passing *qualified* attributes
to `XmlReportWriter::extraToString()` would be a better choice then?
---
-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant-ivy issue #73: Enable XML report parser to produce qualified extra attri...
Github user aprelev commented on the issue: https://github.com/apache/ant-ivy/pull/73 > Tests have passed https://builds.apache.org/job/Ivy-GithubPR/361/ Not really, my changes were not applied in this build. Test *testDepsWithAttributesChanged* should have run under *org.apache.ivy.ant/IvyResolveTest* suite. --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant-ivy issue #73: Enable XML report parser to produce qualified extra attri...
Github user twogee commented on the issue: https://github.com/apache/ant-ivy/pull/73 @aprelev Rather than hardcoding prefix, shouldn't we retrieve it by URL "http://ant.apache.org/ivy/extra";? --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant-ivy issue #73: Enable XML report parser to produce qualified extra attri...
Github user jaikiran commented on the issue: https://github.com/apache/ant-ivy/pull/73 Tests have passed https://builds.apache.org/job/Ivy-GithubPR/361/ --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant-ivy issue #73: Enable XML report parser to produce qualified extra attri...
Github user jaikiran commented on the issue: https://github.com/apache/ant-ivy/pull/73 >> By the way, latest commits 43ddccb and 5918182 introduced nested "javaversion" element, which breaks Jenkins CI build; I had to remove this for both I just fixed the PR build to use the latest 1.9.x version of Ant to build the project. --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant-ivy issue #73: Enable XML report parser to produce qualified extra attri...
Github user jaikiran commented on the issue: https://github.com/apache/ant-ivy/pull/73 retest this please --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant-ivy issue #73: Enable XML report parser to produce qualified extra attri...
Github user aprelev commented on the issue: https://github.com/apache/ant-ivy/pull/73 By the way, latest commits 43ddccb859b94c79350ece7520af4c991c2bb5e6 and 5918182e0d6836d89c42260da9de4428d4cbcec0 introduced [nested "javaversion" element](https://github.com/apache/ant-ivy/blob/5918182e0d6836d89c42260da9de4428d4cbcec0/build.xml#L29), which breaks Jenkins CI build; I had to remove this for both - OpenJDK 1.8.0_171, Ant 1.9.6 (July 8, 2015), Ubuntu 16.04, and - Java 1.8.0_181, Ant 1.9.6 (April 16, 2016), Windows 10.1709 in order to compile Ivy. --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
