TomDmitriev / gradle-bundle-plugin

Apache License 2.0
47 stars 24 forks source link

Disable embedding all project properties within MANIFEST.MF #33

Closed nedtwigg closed 8 years ago

nedtwigg commented 9 years ago

In BundleUtils.getProperties, there is this this little snippet:

        def stringProps = jarTask.project.properties.findAll {
            it.value instanceof String
        }
        attrs + entries + stringProps

I use my gradle.properties to list versions and miscellaneous configuration stuff, and I don't want it to end up in my manifest. Is there a way to disable this?

I guess the point of it is to mark the build with everything that might have affected it?

TomDmitriev commented 9 years ago

Do the properties from your gradle.properties file end up in the bundle manifest file? It is not supposed to work this way. Could you share a repro?

On 19 April 2015 at 14:17, nedtwigg notifications@github.com wrote:

In BundleUtils.getProperties, there is this this little snippet:

    def stringProps = jarTask.project.properties.findAll {
        it.value instanceof String
    }
    attrs + entries + stringProps

I use my gradle.properties to list versions and miscellaneous configuration stuff, and I don't want it to end up in my manifest. Is there a way to disable this?

I guess the point of it is to mark the build with everything that might have affected it?

— Reply to this email directly or view it on GitHub https://github.com/TomDmitriev/gradle-bundle-plugin/issues/33.

Yours sincerely, Artyom Dmitriev

nedtwigg commented 9 years ago

Absolutely! Here is my super-simple test project (just one package with two classes). It builds if you'd like to use it as a test case.

This is my gradle.properties, and this is the MANIFEST.MF it's generating, with the properties at the bottom.

Maybe everyone else is using your plugin with a root project, where none of the subprojects have properties?

Offtopic idea: I use this snippet to keep myself and Eclipse up-to-date with what's going on in the manifest. Might be nice to enable something similar inside the plugin.

TomDmitriev commented 9 years ago

Thanks! Well, the story is that I copied that particular piece of code without much thinking from the original OSGI gradle plugin when I started working on this plugin. Shame on me... But apparently that was how it was supposed to work. Anyway, as you might've noticed not every property goes to the manifest file. The rule is if the name of a property starts with a capital letter, the property is considered to be a manifest header (like this one Bundle-Description=bla-bla). So, in your case you can simply convert the names of your props into lower case to make sure they won't end up in the manifest.

rotty3000 commented 9 years ago

Another alternative, if it's impossible to change these properties, is to use

-removeheaders: VER_.*

nedtwigg commented 9 years ago

Thanks very much! I used instruction '-removeheaders', 'VER_*' (it didn't work with .*).

Would you be open to a pull-request that removes the jarTask.project.properties.findAll line? If anyone is relying on this feature it would break their builds, but that's what a changelog and bumps in the major number are for.

And what's the point of this:

jar {
    manifest {
        attributes 'Implementation-Title': 'Bundle Quickstart',     // Will be added to manifest
    }
}

If we're just going to suck in all of the capitalized properties anyway?

We use Wuff internally, but we're looking to move away because we've had to submit too many PR's that add flags to remove "magic", and the rest of the users prefer to add more magic (totally fine of course, just our company preference :).

TomDmitriev commented 9 years ago

I'd prefer to keep it the way it is to preserve compatibility allowing jar.manifest.attributes to be another way to put attrs into the manifest.

On 21 April 2015 at 22:39, nedtwigg notifications@github.com wrote:

Thanks very much! I used instruction '-removeheaders', 'VER_' (it didn't work with .).

Would you be open to a pull-request that removes the jarTask.project.properties.findAll line? If anyone is relying on this feature it would break their builds, but that's what a changelog and bumps in the major number are for.

And what's the point of this:

jar { manifest { attributes 'Implementation-Title': 'Bundle Quickstart', // Will be added to manifest } }

If we're just going to suck in all of the capitalized properties anyway?

We use Wuff internally, but we're looking to move away because we've had to submit too many PR's that add flags to remove "magic", and the rest of the users prefer to add more magic (totally fine of course, just our company preference :).

— Reply to this email directly or view it on GitHub https://github.com/TomDmitriev/gradle-bundle-plugin/issues/33#issuecomment-94915595 .

Yours sincerely, Artyom Dmitriev

nedtwigg commented 9 years ago

Roger! We will probably fork it just for our own projects at some point in the future. I am very opinionated about unexpected behavior and using semantic versioning to fix broken things without hurting anyone accidentally, but that's my problem, not yours :-)

Thanks for the great plugin!

xtracoder commented 8 years ago

To my mind that code

        def stringProps = jarTask.project.properties.findAll {
            it.value instanceof String
        }
        attrs + entries + stringProps

is harmful because various properties occasionally may be treated as instructions for BndTools. I vote for removing it.

BTW, i can't fund 'that particular piece of code ... in the original OSGI gradle plugin', at least in current version.

xtracoder commented 8 years ago

And .. it would be nice to have a 'bundle-plugin' option to trace all parameters send to Bnd tools from plugin - for diagnostics and troubleshooting.

nedtwigg commented 8 years ago

@xtracoder you might wanna look at osgibnd-gradle-plugin. I don't know if it can do everything that this plugin can, but it makes a MANIFEST and jar using bnd, and it doesn't suck in every property automatically.

TomDmitriev commented 8 years ago

@xtracoder, ok, I'll make the inclusion of the properties optional.

On 20 December 2015 at 16:22, xtracoder notifications@github.com wrote:

To my mind that code

    def stringProps = jarTask.project.properties.findAll {
        it.value instanceof String
    }
    attrs + entries + stringProps

is harmful because various properties occasionally may be treated as instructions for BndTools. I vote for removing it.

BTW, i can't fund 'that particular piece of code ... in the original OSGI gradle plugin', at least in current version.

— Reply to this email directly or view it on GitHub https://github.com/TomDmitriev/gradle-bundle-plugin/issues/33#issuecomment-166116941 .

Yours sincerely, Artyom Dmitriev

TomDmitriev commented 8 years ago

To prevent the project properties from having been passed to Bnd one can do the following.

bundle {
    passProjectProperties = false
}