You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Konrad Windszus <ko...@gmx.de> on 2020/01/05 12:16:16 UTC

OSGi Installer JCR Provider - Dependency on jcr-commons?

Hi and happy new year to everyone,
I am currently bringing the OSGi installer to the newest parent and therefore reviewing the dependencies.
In https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/blob/914ef527268860dd85082ad0c2afe86cd284b1b6/src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrInstaller.java#L783 <https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/blob/914ef527268860dd85082ad0c2afe86cd284b1b6/src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrInstaller.java#L783> a file based node is created manually. Instead I would rather like to use https://jackrabbit.apache.org/api/2.18/org/apache/jackrabbit/commons/JcrUtils.html#putFile-javax.jcr.Node-java.lang.String-java.lang.String-java.io.InputStream- <https://jackrabbit.apache.org/api/2.18/org/apache/jackrabbit/commons/JcrUtils.html#putFile-javax.jcr.Node-java.lang.String-java.lang.String-java.io.InputStream->. For that I would need to add an additional dependency towards jcr-commons to the pom.xml which would lead to another runtime dependency. IMHO that wouldn't be a problem for Sling Starter as that anyways comes with jcr-commons in Startlevel 15 (https://github.com/apache/sling-org-apache-sling-starter/blob/9898f313147e97e15721b71198c5196d4b4a1262/src/main/provisioning/sling.txt#L68 <https://github.com/apache/sling-org-apache-sling-starter/blob/9898f313147e97e15721b71198c5196d4b4a1262/src/main/provisioning/sling.txt#L68>).
Are there any concerns with adding that dependency to the JCR Provider?

Thanks and regards
Konrad


Re: OSGi Installer JCR Provider - Dependency on jcr-commons?

Posted by Carsten Ziegeler <cz...@apache.org>.
We hardcoded the JCR 1.0 dependency years ago to be sure to support JCR 
1.0 regardless of the dependencies. That's probably a legacy artifact 
now and letting bnd do the work seems much better

Having a dependency for just a single simple method seems a little bit 
odd to me, especially as our code doesn't seem to have any defects and 
there is no real advantage of changing it - other than probably to get 
rid of these 15 lines of duplicate code.

It's a trade off, I would prefer leaving it as it is, but adding the 
dependency instead is probably fine as well. I guess in the end it 
doesnt really matter that much.

Carsten

On 05.01.2020 13:19, Konrad Windszus wrote:
> Also supporting JCR 1.0 via https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/blob/914ef527268860dd85082ad0c2afe86cd284b1b6/pom.xml#L68 seems weird to me and I would like to get rid of that and let bnd determine import ranges automatically.
> WDYT?
> 
>> On 5. Jan 2020, at 13:16, Konrad Windszus <ko...@gmx.de> wrote:
>>
>> Hi and happy new year to everyone,
>> I am currently bringing the OSGi installer to the newest parent and therefore reviewing the dependencies.
>> In https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/blob/914ef527268860dd85082ad0c2afe86cd284b1b6/src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrInstaller.java#L783 <https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/blob/914ef527268860dd85082ad0c2afe86cd284b1b6/src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrInstaller.java#L783> a file based node is created manually. Instead I would rather like to use https://jackrabbit.apache.org/api/2.18/org/apache/jackrabbit/commons/JcrUtils.html#putFile-javax.jcr.Node-java.lang.String-java.lang.String-java.io.InputStream- <https://jackrabbit.apache.org/api/2.18/org/apache/jackrabbit/commons/JcrUtils.html#putFile-javax.jcr.Node-java.lang.String-java.lang.String-java.io.InputStream->. For that I would need to add an additional dependency towards jcr-commons to the pom.xml which would lead to another runtime dependency. IMHO that wouldn't be a problem for Sling Starter as that anyways comes with jcr-commons in Startlevel 15 (https://github.com/apache/sling-org-apache-sling-starter/blob/9898f313147e97e15721b71198c5196d4b4a1262/src/main/provisioning/sling.txt#L68 <https://github.com/apache/sling-org-apache-sling-starter/blob/9898f313147e97e15721b71198c5196d4b4a1262/src/main/provisioning/sling.txt#L68>).
>> Are there any concerns with adding that dependency to the JCR Provider?
>>
>> Thanks and regards
>> Konrad
>>
> 

-- 
--
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org

Re: OSGi Installer JCR Provider - Dependency on jcr-commons?

Posted by Konrad Windszus <ko...@gmx.de>.
Also supporting JCR 1.0 via https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/blob/914ef527268860dd85082ad0c2afe86cd284b1b6/pom.xml#L68 seems weird to me and I would like to get rid of that and let bnd determine import ranges automatically.
WDYT?

> On 5. Jan 2020, at 13:16, Konrad Windszus <ko...@gmx.de> wrote:
> 
> Hi and happy new year to everyone,
> I am currently bringing the OSGi installer to the newest parent and therefore reviewing the dependencies.
> In https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/blob/914ef527268860dd85082ad0c2afe86cd284b1b6/src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrInstaller.java#L783 <https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/blob/914ef527268860dd85082ad0c2afe86cd284b1b6/src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrInstaller.java#L783> a file based node is created manually. Instead I would rather like to use https://jackrabbit.apache.org/api/2.18/org/apache/jackrabbit/commons/JcrUtils.html#putFile-javax.jcr.Node-java.lang.String-java.lang.String-java.io.InputStream- <https://jackrabbit.apache.org/api/2.18/org/apache/jackrabbit/commons/JcrUtils.html#putFile-javax.jcr.Node-java.lang.String-java.lang.String-java.io.InputStream->. For that I would need to add an additional dependency towards jcr-commons to the pom.xml which would lead to another runtime dependency. IMHO that wouldn't be a problem for Sling Starter as that anyways comes with jcr-commons in Startlevel 15 (https://github.com/apache/sling-org-apache-sling-starter/blob/9898f313147e97e15721b71198c5196d4b4a1262/src/main/provisioning/sling.txt#L68 <https://github.com/apache/sling-org-apache-sling-starter/blob/9898f313147e97e15721b71198c5196d4b4a1262/src/main/provisioning/sling.txt#L68>).
> Are there any concerns with adding that dependency to the JCR Provider?
> 
> Thanks and regards
> Konrad
>