Get jar label from manifest stamp when present #716
Conversation
| private def getJarLabel(jarPath: String): Option[String] = { | ||
| // First check the jar's manifest for a stamped label | ||
| Try(new JarFile(jarPath)).toOption.flatMap { jf ⇒ | ||
| try { |
There was a problem hiding this comment.
can we change to:
try {
val jf = new JarFile(jarPath)
...rather than Try().toOption since we have a try following anyway? Seems a bit clearer
|
looks like this breaks some tests we have around unused dependencies. Can you inspect those errors and see about fixing those tests? It may be that the test is over strict, or it may be that something really broke. |
|
I’ll take a look at the impl but in general this is definitely something I
want to do (switch to label lookup from manifest rather than passing
labels).
Note that we need to use stamp_jar in scala_import and scala macro library.
Also this will break some tests which talk about exports but maybe that’s
the price we need to pay
…On Mon, 18 Mar 2019 at 22:55 P. Oscar Boykin ***@***.***> wrote:
looks like this breaks some tests we have around unused dependencies.
Can you inspect those errors and see about fixing those tests? It may be
that the test is over strict, or it may be that something really broke.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#716 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIF0Oj91xa5zR8Pn8xGr4wRvLpvD37ks5vX_0mgaJpZM4b6lv4>
.
|
|
@ittaiz is there someone on your end that can look at this? I know the skylark label propagation is something you all wanted. It would be great to switch to the same approach that java is using. cc @andyscott @long-stripe if you are looking for clean up tasks. :) |
|
We wanted it but due to disagreements with upstream java people we've dropped the usage. Essentially we allow label re-mapping (via export) whereas they determine the label at point of creation. @or-shachar maybe this is a relevant task for Shay? |
|
I think I’ll have someone in a month. If Long or Andy want to give it a crack I’ll be happy to guide them |
liucijus
added
the
dep-tracking
* Use module names as package names * Merge previously separate filter validation tests * Move non-version specific tests to org.jacoco.core.test
|
closing as stale, feel free to reopen |
5 participants
This PR gets a jar's label from the 'Target-Label' MANIFEST.MF attribute when available (this is what
java_common.{run_ijar,stamp_jar}both populate).