ijar: fix manifest sections handling

[liucijus]

Fixes #12730

Important changes:

• removed line stripping from the original implementation
• label/rule attributes are appended after main attributes, then the rest of the MANIFEST.MF content is appended - this way sections data is preserved.

[ittaiz]

@cushon wdyt? Rather small PR which could help us out at rules_scala

[ittaiz]

Can anyone help with this? @liucijus stepped up and not only reported the issue but also sent a small PR and it's just sitting here.
FYI we're copying ~25k LOC into rules scala to be unblocked.
Divergences are happening :(

[ittaiz]

@aiuto process wise and bazel community- how long should a community contributor wait before getting a review? Especially for a small and confined PR?
I'm asking because this is demoralizing and works against your efforts of having not only Google look out for Bazel

[comius]

LGTM, except that I would like to also have a test where Target-Label is in the manifest with sections and it is skipped.

[comius]

For reference:

Archive:  jar-with-manifest-sections.jar
  Length      Date    Time    Name
---------  ---------- -----   ----
     1382  2010-01-01 05:00   AnnotatedClass.class
      476  2010-01-01 05:00   Annotations.class
      381  2010-01-01 05:00   Annotations$ParametersOnlyAnnotation.class
      468  2010-01-01 05:00   Annotations$RuntimeInvisible.class
      561  2010-01-01 05:00   Annotations$RuntimeVisible.class
        0  2010-01-01 05:00   META-INF/
       96  2021-01-04 09:23   META-INF/MANIFEST.MF
       19  2018-03-29 15:53   textfile.txt
---------                     -------
     3383                     8 files

$ cat META-INF/MANIFEST.MF 
Manifest-Version: 1.0
Created-By: test-code

Name: foo
Foo: bar

Name: baz
Another: bar

[comius]

@cushon, do you think this could have any impact on the other Java code? My best guess is that it doesn't, because before the patch manifests with sections would just get mangled.

[comius]

For reference:

Archive:  jar-with-target-label-and-manifest-sections.jar
  Length      Date    Time    Name
---------  ---------- -----   ----
     1382  2010-01-01 05:00   AnnotatedClass.class
      476  2010-01-01 05:00   Annotations.class
      381  2010-01-01 05:00   Annotations$ParametersOnlyAnnotation.class
      468  2010-01-01 05:00   Annotations$RuntimeInvisible.class
      561  2010-01-01 05:00   Annotations$RuntimeVisible.class
        0  2010-01-01 05:00   META-INF/
      122  2021-01-29 08:19   META-INF/MANIFEST.MF
       19  2018-03-29 15:53   textfile.txt
---------                     -------
     3409                     8 files

$ cat META-INF/MANIFEST.MF 
Manifest-Version: 1.0
Created-By: test-code
Target-Label: //not:this

Name: foo
Foo: bar

Name: baz
Another: bar

[comius]

Internal linter says: ".jar files are no longer allowed in third_party." and prevents me from adding new .jar file (while old ones are fine). Bazel should be able to generate those .jars from sources, but it's another curve ball.

[liucijus]

LGTM, except that I would like to also have a test where Target-Label is in the manifest with sections and it is skipped.

I assume you want a test to show that existing label is update, right? If I'm not missing anything, it's the way ijar works.

[comius]

LGTM, except that I would like to also have a test where Target-Label is in the manifest with sections and it is skipped.

I assume you want a test to show that existing label is update, right? If I'm not missing anything, it's the way ijar works.

Correct. I already reviewed it, it's ok.

[comius]

Internal linter says: ".jar files are no longer allowed in third_party." and prevents me from adding new .jar file (while old ones are fine). Bazel should be able to generate those .jars from sources, but it's another curve ball.

Do you have resource/will to fix this as well?

[liucijus]

Internal linter says: ".jar files are no longer allowed in third_party." and prevents me from adding new .jar file (while old ones are fine). Bazel should be able to generate those .jars from sources, but it's another curve ball.
Do you have resource/will to fix this as well?

I'll look into it

[liucijus]

Internal linter says: ".jar files are no longer allowed in third_party." and prevents me from adding new .jar file (while old ones are fine). Bazel should be able to generate those .jars from sources, but it's another curve ball.
Do you have resource/will to fix this as well?

I'll look into it

Done

[ittaiz]

Thanks @liucijus and @comius!