search

amclin / aem-packager

A node plugin that creates AEM packages installable through the Adobe Experience Manager package manager.
MIT License
16 stars 4 forks source link

feat: jackrabbit package plugin #489

Open friendlymahi opened 1 year ago

friendlymahi commented 1 year ago

Fixes #488

Proposed Changes

Screenshots and Logs

Before

Error mentioned in #488

After

Build went through fine without any issues

Background context

As per Adobe's documentation Content Package Maven Plugin is no longer supported, and needs replacement with Jackrabbit Package Maven Plugin.

How should this be manually tested?

Create a new AEM project with react as frontend module type. Use the command npm pack in the code obtained from this PR, and place it under node_modules folder of ui.frontend. Then update the npm build script to react-scripts build && clientlib && aem-packager

Questions:

friendlymahi commented 1 year ago

@amclin - Requesting you to review this issue and PR, and if everything looks good, please merge the PR. Thanks !

friendlymahi commented 1 year ago

@amclin - hope you are doing well. can you please review this PR as I would like to use this package for our teams where aem or Java development is not required. If not feasible please let me know so I can copy over the code to our internal module and remove it once you merge the PR. Thanks

amclin commented 1 year ago

@friendlymahi this is a breaking change - it breaks the ability to continue using CRX package deployments

Can you try including both plugins, so that CRX package deployment can still be supported without causing a breaking change? https://jackrabbit.apache.org/filevault-package-maven-plugin/migrating.html

Also, please reenable validation. If maven is throwing errors, this project should not be suppressing them.

friendlymahi commented 1 year ago

@amclin I tested this change and it works fine. Let me explain a bit.

  1. Regarding multiple pluguns - Existing plug-in is failing to locate some classes and as a result the zip file is not created. And we can't have both plug-ins doing the same task. So the build will fail. One option I can think of is, creating an additional maven profile and letting the users provide maven profile also in the configuration if they want to use the new plug in. But regardless I will include both and see what happens and post an update.
  2. Regarding validator settings - Typically in a regular aem maven project we provide repository structure stating what roots are allowed. But here since pom file is coming from this library we can't provide it unless we are ready to make addl code changes. And that too the repository structure or the filter root generation logic should be dependent on the name or npm package name attribute. Now we know that this npm module only generates an independent package without having entire maven project. So validating it's structure and failing the build process is not reasonable. However a warning provides some information to the developer consuming this. Also this strict structure validation is not available in old version of the maven plug in.

Please let me know what you think.

Thanks

amclin commented 1 year ago

Existing plug-in is failing to locate some classes and as a result the zip file is not created.

@friendlymahi this is something specific to your corporate environment, and not a result of using the content-package-maven-plugin. The plugin works just fine with Maven 3.9.3 and OpenJDK 21, as well as far back with the older versions of Java and Maven that aem-packager was originally built with. I don't know what your corporate environment is doing that is blocking the plugin's dependencies, as I said, I recall seeing something in the past about weirdness with expired SSL certs on Adobe's registries that caused issues with certain versions of Java, but ultimately that issue isn't within the scope of aem-packager to be able to solve, and making a breaking change for all consumers to work around your particular corporate environment limitation certainly isn't good practice.

That said, I am open to adding Jackrabbit for the benefits it brings, and making a breaking change for that. I have an idea about how to handle this in a way that gives consumers who need CRX Deployments the ability to continue doing so. Let me see what I can put together for this. I may need your help to test since I don't currently have access to an AEM instance to work with.

friendlymahi commented 1 year ago

sure I can work with you on this. Here is what I did. I used Adobe's latest maven plugin which is v1.0.4. When doing so the zip does get created. However the filter.xml is missing in it. In other words the package becomes unusable for installing via CRX Package Manager. I tested this on my personal Mac which is not on any corp network. And I understand that probably because you dont have a local AEM instance you cant validate the package. I havent tried this, but see if this alternative helps without AEM in place - https://github.com/apache/sling-org-apache-sling-app-cms?tab=readme-ov-file.

One part that I am unclear is consumers who need CRX Deployments the ability to continue doing so, the npm package only asists with creation of zip but not install on a target AEM instance even though the Adobe's plugin is capable of it i.e. the npm package doesnt take any params to support auto deployment using addl maven params.

friendlymahi commented 1 year ago

@amclin - Made my final changes including the content package maven plugin with latest version. Validations are now just excluded for jcrPath since that is unique for every application and cant be assumed without configuring through one of our available parameters or a new one. So made it a warn instead of error. This should keep it simple and minimal. If everything looks good, please merge the PR. Else let me know for more changes. Thanks !

amclin commented 1 year ago

Thanks @friendlymahi, the warn-based approach makes sense.

Can you take a look at this branch please: https://github.com/amclin/aem-packager/tree/feat/jackrabbit

By default it does what you are looking for. But also, I would appreciate if you can test the new option "legacyCRXSupport": true which creates the version with the jar value added for CRX support.

If both of those are functioning, then I'll get this merged in. The approach I'm using here will let me fix some other bugs that have been bothering me.

friendlymahi commented 1 year ago

@amclin - Pulled your changes to my branch and validated the same. Variable mappings were missing, and I fixed them too. Please take a look at the last commit I did. Validated all the 3 usecases (default, jcrvault, jackrabbit+crxCompatibility), and pacjkages are created as expected. However I have no clue on the usage of content-package-maven-plugin when we are not installing to an AEM instance. So except for that all other use cases are validated. If all good, please merge the PR, and this shall automatically merge your PR as well I guess. Thanks !

friendlymahi commented 1 year ago

@amclin - Are the changes good? Looking for new version so I can use this at my work for a demo. Thanks !

friendlymahi commented 1 year ago

@amclin - Any luck with the review? Thanks

friendlymahi commented 1 year ago

@amclin - Hope you are doing well. Can you please help moving this PR if everything is okay? We recently upgraded our AEM environment and want to validate the module against the new version of AEM we have. Thanks!

friendlymahi commented 10 months ago

Hello @amclin

Hope you are doing well.

Can you please confirm please review and merge this PR? if not feasible, I will fork the code and go with my custom solution as this is long overdue for my work.

Thanks in advance, Mahidhar Ch