Skip to content

Commit e0e1398

Browse files
lostlucklostluck
authored
[#25582] Use a Pathing Jar instead of long command line class paths in Java Boot loader. (#26087)
1 parent 50c6d04 commit e0e1398

5 files changed

Lines changed: 235 additions & 46 deletions

File tree

‎.github/workflows/go_tests.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ on:
2828
pull_request:
2929
branches: ['master', 'release-*']
3030
tags: ['v*']
31-
paths: ['sdks/go/pkg/**', 'sdks/go.mod', 'sdks/go.sum']
31+
paths: ['sdks/go/pkg/**', 'sdks/go.mod', 'sdks/go.sum', 'sdks/go/container/*', 'sdks/java/container/*', 'sdks/python/container/*', 'sdks/typescript/container/*']
3232
# This allows a subsequently queued workflow run to interrupt previous runs
3333
concurrency:
3434
group: '${{ github.workflow }} @ ${{ github.event.pull_request.head.label || github.head_ref || github.ref }}'
@@ -48,7 +48,7 @@ jobs:
4848
- name: Delete old coverage
4949
run: "cd sdks/go/pkg && rm -rf .coverage || :"
5050
- name: Run coverage
51-
run: cd sdks/go/pkg && go test -coverprofile=coverage.txt -covermode=atomic ./...
51+
run: cd sdks && go test -coverprofile=coverage.txt -covermode=atomic ./go/pkg/... ./go/container/... ./java/container/... ./python/container/... ./typescript/container/...
5252
- uses: codecov/codecov-action@v3
5353
with:
5454
flags: go

‎sdks/java/container/boot.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,6 @@ func main() {
165165
"-XX:+UseParallelGC",
166166
"-XX:+AlwaysActAsServerClassMachine",
167167
"-XX:-OmitStackTraceInFastThrow",
168-
"-cp", strings.Join(cp, ":"),
169168
}
170169

171170
enableGoogleCloudProfiler := strings.Contains(options, enableGoogleCloudProfilerOption)
@@ -210,9 +209,14 @@ func main() {
210209

211210
// (2) Add classpath: "-cp foo.jar:bar.jar:.."
212211
if len(javaOptions.Classpath) > 0 {
213-
args = append(args, "-cp")
214-
args = append(args, strings.Join(javaOptions.Classpath, ":"))
212+
cp = append(cp, javaOptions.Classpath...)
215213
}
214+
pathingjar, err := makePathingJar(cp)
215+
if err != nil {
216+
logger.Fatalf(ctx, "makePathingJar failed: %v", err)
217+
}
218+
args = append(args, "-cp")
219+
args = append(args, pathingjar)
216220

217221
// (3) Add (sorted) properties: "-Dbar=baz -Dfoo=bar .."
218222
var properties []string

‎sdks/java/container/boot_test.go‎

Lines changed: 47 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -18,57 +18,63 @@
1818
package main
1919

2020
import (
21-
"reflect"
22-
"testing"
21+
"context"
22+
"reflect"
23+
"testing"
24+
25+
"github.com/apache/beam/sdks/v2/go/container/tools"
2326
)
2427

2528
func TestBuildOptionsEmpty(t *testing.T) {
26-
dir := "test/empty"
27-
metaOptions, err := LoadMetaOptions(dir)
28-
if err != nil {
29-
t.Fatalf("Got error %v running LoadMetaOptions", err)
30-
}
31-
if metaOptions != nil {
32-
t.Fatalf("LoadMetaOptions(%v) = %v, want nil", dir, metaOptions)
33-
}
29+
ctx, logger := context.Background(), &tools.Logger{}
30+
dir := "test/empty"
31+
metaOptions, err := LoadMetaOptions(ctx, logger, dir)
32+
if err != nil {
33+
t.Fatalf("Got error %v running LoadMetaOptions", err)
34+
}
35+
if metaOptions != nil {
36+
t.Fatalf("LoadMetaOptions(%v) = %v, want nil", dir, metaOptions)
37+
}
3438

35-
javaOptions := BuildOptions(metaOptions)
36-
if len(javaOptions.JavaArguments) != 0 || len(javaOptions.Classpath) != 0 || len(javaOptions.Properties) != 0 {
37-
t.Errorf("BuildOptions(%v) = %v, want nil", metaOptions, javaOptions)
38-
}
39+
javaOptions := BuildOptions(ctx, logger, metaOptions)
40+
if len(javaOptions.JavaArguments) != 0 || len(javaOptions.Classpath) != 0 || len(javaOptions.Properties) != 0 {
41+
t.Errorf("BuildOptions(%v) = %v, want nil", metaOptions, javaOptions)
42+
}
3943
}
4044

4145
func TestBuildOptionsDisabled(t *testing.T) {
42-
metaOptions, err := LoadMetaOptions("test/disabled")
43-
if err != nil {
44-
t.Fatalf("Got error %v running LoadMetaOptions", err)
45-
}
46+
ctx, logger := context.Background(), &tools.Logger{}
47+
metaOptions, err := LoadMetaOptions(ctx, logger, "test/disabled")
48+
if err != nil {
49+
t.Fatalf("Got error %v running LoadMetaOptions", err)
50+
}
4651

47-
javaOptions := BuildOptions(metaOptions)
48-
if len(javaOptions.JavaArguments) != 0 || len(javaOptions.Classpath) != 0 || len(javaOptions.Properties) != 0 {
49-
t.Errorf("BuildOptions(%v) = %v, want nil", metaOptions, javaOptions)
50-
}
52+
javaOptions := BuildOptions(ctx, logger, metaOptions)
53+
if len(javaOptions.JavaArguments) != 0 || len(javaOptions.Classpath) != 0 || len(javaOptions.Properties) != 0 {
54+
t.Errorf("BuildOptions(%v) = %v, want nil", metaOptions, javaOptions)
55+
}
5156
}
5257

5358
func TestBuildOptions(t *testing.T) {
54-
metaOptions, err := LoadMetaOptions("test/priority")
55-
if err != nil {
56-
t.Fatalf("Got error %v running LoadMetaOptions", err)
57-
}
59+
ctx, logger := context.Background(), &tools.Logger{}
60+
metaOptions, err := LoadMetaOptions(ctx, logger, "test/priority")
61+
if err != nil {
62+
t.Fatalf("Got error %v running LoadMetaOptions", err)
63+
}
5864

59-
javaOptions := BuildOptions(metaOptions)
60-
wantJavaArguments := []string{"java_args=low", "java_args=high"}
61-
wantClasspath := []string{"classpath_high", "classpath_low"}
62-
wantProperties := map[string]string{
63-
"priority":"high",
64-
}
65-
if !reflect.DeepEqual(javaOptions.JavaArguments, wantJavaArguments) {
66-
t.Errorf("BuildOptions(%v).JavaArguments = %v, want %v", metaOptions, javaOptions.JavaArguments, wantJavaArguments)
67-
}
68-
if !reflect.DeepEqual(javaOptions.Classpath, wantClasspath) {
69-
t.Errorf("BuildOptions(%v).Classpath = %v, want %v", metaOptions, javaOptions.Classpath, wantClasspath)
70-
}
71-
if !reflect.DeepEqual(javaOptions.Properties, wantProperties) {
72-
t.Errorf("BuildOptions(%v).JavaProperties = %v, want %v", metaOptions, javaOptions.Properties, wantProperties)
73-
}
65+
javaOptions := BuildOptions(ctx, logger, metaOptions)
66+
wantJavaArguments := []string{"java_args=low", "java_args=high"}
67+
wantClasspath := []string{"classpath_high", "classpath_low"}
68+
wantProperties := map[string]string{
69+
"priority": "high",
70+
}
71+
if !reflect.DeepEqual(javaOptions.JavaArguments, wantJavaArguments) {
72+
t.Errorf("BuildOptions(%v).JavaArguments = %v, want %v", metaOptions, javaOptions.JavaArguments, wantJavaArguments)
73+
}
74+
if !reflect.DeepEqual(javaOptions.Classpath, wantClasspath) {
75+
t.Errorf("BuildOptions(%v).Classpath = %v, want %v", metaOptions, javaOptions.Classpath, wantClasspath)
76+
}
77+
if !reflect.DeepEqual(javaOptions.Properties, wantProperties) {
78+
t.Errorf("BuildOptions(%v).JavaProperties = %v, want %v", metaOptions, javaOptions.Properties, wantProperties)
79+
}
7480
}

‎sdks/java/container/pathingjar.go‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one or more
2+
// contributor license agreements. See the NOTICE file distributed with
3+
// this work for additional information regarding copyright ownership.
4+
// The ASF licenses this file to You under the Apache License, Version 2.0
5+
// (the "License"); you may not use this file except in compliance with
6+
// the License. You may obtain a copy of the License at
7+
//
8+
// http://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
16+
package main
17+
18+
import (
19+
"archive/zip"
20+
"fmt"
21+
"io"
22+
"os"
23+
"strings"
24+
)
25+
26+
// makePathingJar produces a context or 'pathing' jar which only contains the relative
27+
// classpaths in its META-INF/MANIFEST.MF.
28+
//
29+
// Since we build with Java 8 as a minimum, this is the only supported way of reducing
30+
// command line length, since argsfile support wasn't added until Java 9.
31+
//
32+
// https://docs.oracle.com/javase/8/docs/technotes/guides/jar/jar.html is the spec.
33+
//
34+
// In particular, we only need to populate the Jar with a Manifest-Version and Class-Path
35+
// attributes.
36+
// Per https://docs.oracle.com/javase/8/docs/technotes/guides/jar/jar.html#classpath
37+
// the classpath URLs must be relative for security reasons.
38+
func makePathingJar(classpaths []string) (string, error) {
39+
f, err := os.Create("pathing.jar")
40+
if err != nil {
41+
return "", fmt.Errorf("unable to create pathing.jar: %w", err)
42+
}
43+
defer f.Close()
44+
if err := writePathingJar(classpaths, f); err != nil {
45+
return "", fmt.Errorf("unable to write pathing.jar: %w", err)
46+
}
47+
return f.Name(), nil
48+
}
49+
50+
var lineBreak = []byte{'\r', '\n'}
51+
var continuation = []byte{' '}
52+
53+
func writePathingJar(classpaths []string, w io.Writer) error {
54+
jar := zip.NewWriter(w)
55+
defer jar.Close()
56+
57+
if _, err := jar.Create("META-INF/"); err != nil {
58+
return fmt.Errorf("unable to create META-INF/ directory: %w", err)
59+
}
60+
61+
zf, err := jar.Create("META-INF/MANIFEST.MF")
62+
if err != nil {
63+
return fmt.Errorf("unable to create META-INF/MANIFEST.MF: %w", err)
64+
}
65+
66+
zf.Write([]byte("Manifest-Version: 1.0"))
67+
zf.Write(lineBreak)
68+
zf.Write([]byte("Created-By: sdks/java/container/pathingjar.go"))
69+
zf.Write(lineBreak)
70+
// Class-Path: must have a sequence of relative URIs for the paths
71+
// which we assume outright in this case.
72+
73+
// We could do this memory efficiently, but it's not worthwhile compared to the complexity
74+
// at this stage.
75+
allCPs := "Class-Path: file:" + strings.Join(classpaths, " file:")
76+
77+
const lineLenMax = 71 // it's actually 72, but we remove 1 to account for the continuation line prefix.
78+
buf := make([]byte, lineLenMax)
79+
cur := 0
80+
for cur+lineLenMax < len(allCPs) {
81+
next := cur + lineLenMax
82+
copy(buf, allCPs[cur:next])
83+
zf.Write(buf)
84+
zf.Write(lineBreak)
85+
zf.Write(continuation)
86+
cur = next
87+
}
88+
zf.Write([]byte(allCPs[cur:]))
89+
zf.Write(lineBreak)
90+
return nil
91+
}

‎sdks/java/container/pathingjar_test.go‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one or more
2+
// contributor license agreements. See the NOTICE file distributed with
3+
// this work for additional information regarding copyright ownership.
4+
// The ASF licenses this file to You under the Apache License, Version 2.0
5+
// (the "License"); you may not use this file except in compliance with
6+
// the License. You may obtain a copy of the License at
7+
//
8+
// http://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
16+
package main
17+
18+
import (
19+
"archive/zip"
20+
"bufio"
21+
"bytes"
22+
"io"
23+
"strings"
24+
"testing"
25+
)
26+
27+
func TestWritePathingJar(t *testing.T) {
28+
var buf bytes.Buffer
29+
input := []string{"a.jar", "b.jar", "c.jar", "thisVeryLongJarNameIsOverSeventyTwoCharactersLongAndNeedsToBeSplitCorrectly.jar"}
30+
err := writePathingJar(input, &buf)
31+
if err != nil {
32+
t.Errorf("writePathingJar(%v) = %v, want nil", input, err)
33+
}
34+
35+
zr, err := zip.NewReader(bytes.NewReader(buf.Bytes()), int64(buf.Len()))
36+
if err != nil {
37+
t.Errorf("zip.NewReader() = %v, want nil", err)
38+
}
39+
40+
f := getManifest(zr)
41+
if f == nil {
42+
t.Fatalf("Jar didn't contain manifest")
43+
}
44+
45+
{
46+
fr, err := f.Open()
47+
if err != nil {
48+
t.Errorf("(%v).Open() = %v, want nil", f.Name, err)
49+
}
50+
all, err := io.ReadAll(fr)
51+
if err != nil {
52+
t.Errorf("(%v).Open() = %v, want nil", f.Name, err)
53+
}
54+
fr.Close()
55+
want := "\nClass-Path: file:a.jar file:b.jar file:c.jar"
56+
if !strings.Contains(string(all), want) {
57+
t.Errorf("%v = %v, want nil", f.Name, err)
58+
}
59+
}
60+
61+
{
62+
fr, err := f.Open()
63+
if err != nil {
64+
t.Errorf("(%v).Open() = %v, want nil", f.Name, err)
65+
}
66+
defer fr.Close()
67+
fs := bufio.NewScanner(fr)
68+
fs.Split(bufio.ScanLines)
69+
70+
for fs.Scan() {
71+
if got, want := len(fs.Bytes()), 72; got > want {
72+
t.Errorf("Manifest line exceeds limit got %v:, want %v line: %q", got, want, fs.Text())
73+
}
74+
}
75+
}
76+
77+
}
78+
79+
// getManifest extracts the java manifest from the zip file.
80+
func getManifest(zr *zip.Reader) *zip.File {
81+
for _, f := range zr.File {
82+
if f.Name != "META-INF/MANIFEST.MF" {
83+
continue
84+
}
85+
return f
86+
}
87+
return nil
88+
}

0 commit comments

Comments
 (0)