Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

java_binary: input MANIFEST file is overridden with the MANIFEST content from dependent JARs #44

Closed
davido opened this issue Aug 30, 2013 · 7 comments

Comments

@davido
Copy link
Contributor

davido commented Aug 30, 2013

BUCK uses jar -cfm command to assemble the java_binary.

If the dependent JARs also contain MANIFEST.MF file then jar command mess up the content from input MANIFEST.MF and dependent JARs MANIFEST.MF files.

See that change from Gerrit project for more details:

https://gerrit-review.googlesource.com/49305

@shs96c
Copy link
Contributor

shs96c commented Sep 15, 2013

I'm surprised. From the entry you link to, you say that the manifest isn't properly formed. What's the best way to repro the issue? Just run that target? Which entry in particular are you expecting to see in the manifest?

The code which generates the final jar for a java_binary can be seen here:

https://github.com/facebook/buck/blob/master/src/com/facebook/buck/java/JarDirectoryStep.java

In "createJarFile" we pass in a Manifest, which is updated (in "merge(Manifest, Manifest)") as each dependent jar is read. The final thing that happens is that we write the manifest entry to the jar. The end result is that the manifest in the generated jar should be the combined manifest.

As you can also see, the "getDescription" method takes a few liberties about how we represent this process. :)

@davido
Copy link
Contributor Author

davido commented Sep 15, 2013

I created the reproducer for the issue above and detailed instructions in README file how to reproduce it:

https://github.com/davido/buck_test

@shs96c
Copy link
Contributor

shs96c commented Sep 16, 2013

Ah! Now I understand. Yes, it's lame that we do that, and we should fix the behaviour.

@shs96c
Copy link
Contributor

shs96c commented Sep 16, 2013

Getting a patch reviewed for this.

@davido
Copy link
Contributor Author

davido commented Sep 16, 2013

Hey, cool, thanks! Can you share the patch?

@shs96c
Copy link
Contributor

shs96c commented Sep 17, 2013

I've just landed it on our internal repo, and it'll be out in the next MOE push.

@shs96c
Copy link
Contributor

shs96c commented Sep 17, 2013

Marking as "closed" because the code is now in a tree, and just waiting to be pushed out to Real Life.

@shs96c shs96c closed this as completed Sep 17, 2013
ckamm pushed a commit to ckamm/gerrit that referenced this issue Jan 30, 2014
d693330 fixed Buck's bug, that was
reproduced [1], reported [2] and fixed [3] upstream.

[1] https://github.com/davido/buck_test
[2] facebook/buck#44
[3] facebook/buck@e93cb2a

Change-Id: Ie6cfa832c8c9dc100ff6fc63c5583aa931857359
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants