CCing David and Sergey. Some changes are in web.project and j2ee.persistence so feel free to comment on the patch (I'm going to attach it in a few minutes).
If there will be no objections, I'll integrate the patch tomorrow.

Created attachment 135987 [details]
Resolving EMGenStrategyResolverImpl

In the patch I'm moving EMGenStrategyResolverImpl class from j2ee.common to the j2ee.persistence. In the persistence module I have created new api package (corresponding to the already existing spi) which contains only one factory class that returns default instance of EMGenStrategyResolverImpl.

CCing also MartinF, I have made tiny changes in j2ee.ejbjarproject too.

changes may affect maven projects, may be in maven.persistence module but I'll check better tomorrow.

Thanks Martin. Unsorted comments:

* j2ee.persistence already has two API packages named "entitygenerator" and "api.entity.generator". Adding third one named "api.entitymanagergenerator" sound suspicious. :-) I did not look though into existing packages and what's in them.

* spec version could be just 1.39 (instead of 1.39.1)

* new API method "createInstance" could be annotated in its Javadoc "@since 1.39"

Otherwise this looks good.

I was surprised to to discover "ContainerClassPathModifier" API class! It is first time I've ever seen it. It has a single impl in maven.j2ee and it looks it does nothing since EE6. I think even for EE5 it probably could be deleted as Maven dependencies are now handled via Ant libraries which have POM reference.

(In reply to comment #5)
> Thanks Martin. Unsorted comments:
> 
> * j2ee.persistence already has two API packages named "entitygenerator" and
> "api.entity.generator". Adding third one named "api.entitymanagergenerator"
> sound suspicious. :-)

I would like to let the decision up to Sergey as the module owner. I created "api.entitymanagergenerator" because the related SPI class is placed in "spi.entitymanagergenerator" but not sure if such convention has to be respected.

> * spec version could be just 1.39 (instead of 1.39.1)
> 
> * new API method "createInstance" could be annotated in its Javadoc "@since
> 1.39"

Ok, I'll do that

> I was surprised to to discover "ContainerClassPathModifier" API class! It is
> first time I've ever seen it. It has a single impl in maven.j2ee and it looks
> it does nothing since EE6. I think even for EE5 it probably could be deleted as
> Maven dependencies are now handled via Ant libraries which have POM reference.

I'm a little bit confused. I guess this is unrelated, right? :) ..anyway I agree the implementation looks weird. I'll talk to Milos tomorrow.

entitymanagergenerator and entitygenerator even with similar naming should be different by meaning, also packages are on different level, and both contain a lot of classes, we do not have one-two class per package here. 
I'm ok with entitymanagergenerator package but api.entity.generator and entitygenerator packages may need some review/rename/refactor. Persistence may need spome cleanup from 1.4 support also.

Thanks, I'll stay with current "api.entitymanagergenerator" in that case.

> I'm a little bit confused. I guess this is unrelated, right? :) ..anyway I
> agree the implementation looks weird. I'll talk to Milos tomorrow.

Yeah sorry it is unrelated. :-) I noticed the class while looking at your diff and was surprised I've never seen it before.

(In reply to comment #8)
> Thanks, I'll stay with current "api.entitymanagergenerator" in that case.

After discussion with PetrH (considering him as one of the most experienced API designer in our team;)), I'll move the Factory class directly to the SPI package "spi.entitymanagergenerator". Feel free to comment if you don't think it's a good idea

Created attachment 136080 [details]
Resolving EMGenStrategyResolverImpl - v2

- @since annotation added
- Factory class moved to SPI package
- spec. version changed to 1.39

> I'll move the Factory class directly to the SPI
> package "spi.entitymanagergenerator".

That's a very good idea.

First part fix in: web-main #6f7a6d2eba46
Plus I had to move javaee.injection into the J2SE cluster (web-main #130cceb8ef54) because j2ee.persistence lies there and depends on it.

> Plus I had to move javaee.injection into the J2SE cluster

It's a detail, but when you have to do something like that it is usually an indicator that we are doing something wrong. Unfortunately. :-)

Sorry I did not pick up on it earlier - I keep forgetting that persistence APIs are defined in J2SE cluster.

On one hand we could leave everything as is. On the other hand if you look at the purpose of javaee.injection and who implements it and how then you can see that nobody will ever use it in J2SE cluster. Similarly if you look at EntityManagerGenerationStrategyResolverFactory.EMGenStrategyResolverImpl and what the javaee.injection is used there for you can see that this implementation makes sense only in Web/EE project types and nobody in J2SE cluster will actually use it. So, inevitable conclusion is that EntityManagerGenerationStrategyResolverFactory.EMGenStrategyResolverImpl does not belong to J2SE cluster. Such problem is fixed usually in two ways:

#1) either keep EntityManagerGenerationStrategyResolverFactory in the J2SE but lookup its implementation via a global lookup; implementation would be then provided via some java ee cluster module (and javaee.injection can go back to ee cluster too); EntityManagerGenerationStrategyResolverFactory fallbacks on some dummy impl if lookup is empty (which would be "use ApplicationManagedResourceTransactionInJ2SE.class strategy"); or

#2) move EntityManagerGenerationStrategyResolverFactory into EE cluster (eg. into javaee.project module which we will create soon)

Integrated into 'main-golden', will be available in build *201306242301* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/6f7a6d2eba46
User: Martin Janicek <mjanicek@netbeans.org>
Log: #230831 - Refactor EMGenStrategyResolverImpl and PersistenceProviderSupplierImpl

(In reply to comment #14)
> So, inevitable conclusion is that
> EntityManagerGenerationStrategyResolverFactory.EMGenStrategyResolverImpl does
> not belong to J2SE cluster.

I agree with you but on the other hand I think this is not a problem of EntityManagerGenerationStrategyResolverFactory but rather the part of the j2ee.persistence which is in SE cluster but does not have anything in common with SE. For example most of the classes from spi.entitymanagergenerator package ends up with -EJB or -Web suffix and obviously doesn't belong to SE.

"good"  design may be to have separate modules for general jpa and jpa-ee, but it may hit user experience to update/configure/enable-disable two separate modules but it may not be a problem. But may require some more work for redesign.

I agree that perhaps persistence should have had J2SE and J2EE parts. That might be non-trivial to do now though.

This old bug may not be relevant anymore. If you can still reproduce it in 8.2 development builds please reopen this issue.

Thanks for your cooperation,
NetBeans IDE 8.2 Release Boss