Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a MakeJar to create a plain-old-jar file as a resource #5918

Closed
wants to merge 1 commit into from

Conversation

laeubi
Copy link
Contributor

@laeubi laeubi commented Dec 2, 2023

Currently one can already create a new bundle using additional bnd instructions file but this has some caveats when one only wants to package some stuff into a jar and also some processing overhead.

This adds a new MakeJar that do what the name suggest and create a dumb jar from an input directory.

@laeubi laeubi force-pushed the make_jar branch 2 times, most recently from 29fe5e0 to b816682 Compare December 3, 2023 19:06
@pkriens
Copy link
Member

pkriens commented Dec 4, 2023

A Jar is created ahead of time. We generally try to delay the work until we actually write the output.

It is also never closed. This is not a big deal in this case because it is a directory but a Jar is an AutoCloseable. It is inside knowledge that I know that a directory is ok, on a Windows a Jar on a file locks it up.

I also do not think we need to add a new type, there is already an applicable MakeCopy.

I think the best solution is to modify FileResource to create a Jar on output time when the file happens to be a directory and then modify MakeCopy to accept directories. Currently a file must be a file and not a dir.

@laeubi
Copy link
Contributor Author

laeubi commented Dec 4, 2023

A Jar is created ahead of time. We generally try to delay the work until we actually write the output.

Good point, even though the JarResource seems to be made for this, what do you suggesting instead? As Bnd at this points really want the resource because it is missing I assume it can't delay much here anyways.

It is also never closed. This is not a big deal in this case because it is a directory but a Jar is an AutoCloseable. It is inside knowledge that I know that a directory is ok, on a Windows a Jar on a file locks it up.

What files should it look up? It does only operate on memory resources here as far as I can see.

modify MakeCopy to accept directories

I'm not sure I understand the MakeCopy to propose something like that, also not I don't want to copy that folder into the (bundle) jar, I want it to pack the contents into a new jar and then put it into the bundle-jar, not sure if MakeCopy really do that?

@pkriens
Copy link
Member

pkriens commented Dec 4, 2023

What files should it look up? It does only operate on memory resources here as far as I can see.

As I said, a folder is ok. However, a zip file is opened as a stream and the entries are in the resource. However, these are internal details you should never rely on. Jar declares AutoCloseable so it must be life cycle managed.

MakeCopy does not handle folders because it is logically impossible to expand the structure in the jar, it can only provide a resource == single entry in the jar.

Currently a folder is invalid and will likely give weird errors since it tries to turn it into a URL. So we can safely turn a folder into a jar/zip entry/Resource. In bnd, that does not sound illogical. Then we could:

  • Use JarResource (and close the Jar)
  • Use FileResource (needs a small change but if done well, I think it makes a lot of sense).
  • Make a special FolderResource

The simplest solution is probably a new FolderResource. I am not fond of the JarResource since Jars are big objects and prefer to have it created when needed. It is generally used when you already have a Jar at hand. We could use FileResource but it is one of the workhorses and BJ added quite a bit of complexity do handle caching and buffers. So a new FolderResource might be the simplest solution.

@laeubi
Copy link
Contributor Author

laeubi commented Dec 4, 2023

As I said, a folder is ok. However, a zip file is opened as a stream and the entries are in the resource.

But that the point the "Jar" is a class it is not a file so it can't have anything opened as it is only a structure in memory ... and if you look at close it closes the zipfile (what is null as the jar is not created from a zip) and it closes all resources , what is a noop as well as we only have fileresources that have a pointer to the file but not the file content and then throws away all added resources so after that it is completely empty so can't be used as a jar anymore... In contrast the JarResource can close the jar.

Nerveless I can change that without a problem, but if you are so concerned you might be interested to fix aQute.bnd.make.MakeBnd what EXACTLY do use the same pattern with a Jar+JarResource, guess how I found about it ;-)

Currently a folder is invalid and will likely give weird errors since it tries to turn it into a URL. So we can safely turn a folder into a jar/zip entry/Resource. In bnd, that does not sound illogical

What I don't understand is what a "FolderResource" should be transformed to a byte stream (what a resource actually is) without specify if it should be jar, gzip, tar, ...

So I still think we probably talk about different things here but not sure... a folder can already be added to a jar as individual (file) resources.

@pkriens
Copy link
Member

pkriens commented Dec 4, 2023

But that the point the "Jar" is a class it is not a file so it can't have anything opened as it is only a structure in memory ...
See ZipResource and Jar.buildFromZip.

we spent months trying to fix issues with this because it blocked builds on Windows. As I said, a folder would be ok but you're not allowed to assume that ever. that is internal knowledge. Follow the API! It says it is an AutoCloseable so you must close it.

Nerveless I can change that without a problem, but if you are so concerned you might be interested to fix aQute.bnd.make.MakeBnd what EXACTLY do use the same pattern with a Jar+JarResource, guess how I found about it ;-)

Not really. See line 36 of MakeBnd:

builder.addClose(bchild);

What I don't understand is what a "FolderResource" should be transformed to a byte stream (what a resource actually is) without specify if it should be jar, gzip, tar, ...

As I said, I think it is logical in bnd to make it a zip. But be my guest to support more formats ... The make copy gets the information to look at the extension of the file name ...

You're costing me way more time then anybody else. The common feeling I get is that you keep second guessing me. It might help is both if you assume I am right in OSGi and bnd issues.

Currently one can already create a new bundle using additional bnd
instructions file but this has some caveats when one only wants to
package some stuff into a jar and also some processing overhead.

This adds a new MakeJar that do what the name suggest and create a dumb
jar from an input directory.

Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
@laeubi
Copy link
Contributor Author

laeubi commented Dec 4, 2023

It might help is both if you assume I am right in OSGi and bnd issues.

I try to not assume anything beforehand, nevertheless if your the only one that is always right its a bit annoying to ask people to open PRs instead of issues just to tell them you don't like the effort put into PRs.... maybe it could also help to not assume everyone else is a complete idiot regarding OSGi + bnd.

Anyways I'm now made the resource completely lazy and Autoclose the jar as soon as possible... and I don't do this to annoy anyone but because I think it might be useful for others.

@pkriens
Copy link
Member

pkriens commented Dec 24, 2023

I've just committed a change, you can now use

-includeresource foo.jar=@some/directory/

@pkriens pkriens closed this Dec 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants