[go] embed: document the maximum file size supported
korzhao (Gerrit)
korzhao has uploaded this change for review.
embed: document the maximum file size supported
Fixes #47627
Change-Id: Ia1edfb6249863ab055fab68a35666bc2bdf21dcb
---
M src/cmd/compile/internal/staticdata/data.go
M src/embed/embed.go
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/cmd/compile/internal/staticdata/data.go b/src/cmd/compile/internal/staticdata/data.go
index abb0bba..98b4ed5 100644
--- a/src/cmd/compile/internal/staticdata/data.go
+++ b/src/cmd/compile/internal/staticdata/data.go
@@ -138,7 +138,7 @@
// and probably the rest of the toolchain
// can't handle such big symbols either.
// See golang.org/issue/9862.
- return nil, 0, fmt.Errorf("file too large")
+ return nil, 0, fmt.Errorf("file is larger than 2 GB")
}
// File is too big to read and keep in memory.
diff --git a/src/embed/embed.go b/src/embed/embed.go
index 851cc21..9170969 100644
--- a/src/embed/embed.go
+++ b/src/embed/embed.go
@@ -7,6 +7,7 @@
// Go source files that import "embed" can use the //go:embed directive
// to initialize a variable of type string, []byte, or FS with the contents of
// files read from the package directory or subdirectories at compile time.
+// The maximum supported file size is 2 GB.
//
// For example, here are three ways to embed a file named hello.txt
// and then print its contents at run time.
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
Attention is currently required from: Ian Lance Taylor, korzhao.
Patch set 1:Run-TryBot +1
1 comment:
File src/embed/embed.go:
Patch Set #1, Line 10:
// The maximum supported file size is 2 GB.Let's move this sentence down near L94. There are a couple other sentences there about what can and can't be embedded, and it makes sense to keep that information together.
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
korzhao (Gerrit)
Attention is currently required from: Ian Lance Taylor, korzhao.
korzhao uploaded patch set #2 to this change.
embed: document the maximum file size supported
Fixes #47627
Change-Id: Ia1edfb6249863ab055fab68a35666bc2bdf21dcb
---
M src/cmd/compile/internal/staticdata/data.go
M src/embed/embed.go
2 files changed, 5 insertions(+), 1 deletion(-)
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
korzhao (Gerrit)
Attention is currently required from: Jay Conrod, Ian Lance Taylor.
1 comment:
File src/embed/embed.go:
Patch Set #1, Line 10:
// The maximum supported file size is 2 GB.
Let's move this sentence down near L94. […]
Done
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
Attention is currently required from: Ian Lance Taylor, korzhao.
1 comment:
File src/embed/embed.go:
Patch Set #1, Line 10:
// The maximum supported file size is 2 GB.
Done
I don't think this is important enough to merit its own section. Let's put it together with the other things that can't be matched near L94.
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
korzhao (Gerrit)
Attention is currently required from: Ian Lance Taylor, korzhao.
korzhao uploaded patch set #3 to this change.
embed: document the maximum file size supported
Fixes #47627
Change-Id: Ia1edfb6249863ab055fab68a35666bc2bdf21dcb
---
M src/cmd/compile/internal/staticdata/data.go
M src/embed/embed.go
2 files changed, 2 insertions(+), 2 deletions(-)
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
korzhao (Gerrit)
Attention is currently required from: Jay Conrod, Ian Lance Taylor.
1 comment:
File src/embed/embed.go:
Patch Set #1, Line 10:
// The maximum supported file size is 2 GB.
I don't think this is important enough to merit its own section. […]
Done
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
Attention is currently required from: Ian Lance Taylor, korzhao.
Patch set 3:Run-TryBot +1
1 comment:
Patchset:
TRY=linux-amd64-longtest,windows-amd64-longtest
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
Attention is currently required from: Ian Lance Taylor, korzhao.
Patch set 3:Code-Review +2
Michael Matloob (Gerrit)
Attention is currently required from: Ian Lance Taylor, korzhao.
Patch set 3:Trust +1
Jay Conrod (Gerrit)
Attention is currently required from: Ian Lance Taylor, korzhao.
Patch set 3:-Code-Review
1 comment:
File src/embed/embed.go:
Patch Set #3, Line 91:
// must match at least one file or non-empty directory. The maximum supported file size is 2 GB.Let's actually drop this part of the change and not document this limit. Users seeing the new error message will at least understand what the limit is.
I asked why we had this limit, and it's actually a toolchain implementation detail. It may change in the future. We also aren't confident the toolchain will behave well close to the limit, for example, embedding 10 1.9GB files may or may not work; this is not well tested.
So let's not commit to anything specific here in the documentation.
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
korzhao (Gerrit)
Attention is currently required from: Ian Lance Taylor, korzhao.
korzhao uploaded patch set #4 to this change.
embed: document the maximum file size supported
Fixes #47627
Change-Id: Ia1edfb6249863ab055fab68a35666bc2bdf21dcb
---
M src/cmd/compile/internal/staticdata/data.go
1 file changed, 1 insertion(+), 1 deletion(-)
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
korzhao (Gerrit)
Attention is currently required from: Jay Conrod, Ian Lance Taylor.
1 comment:
File src/embed/embed.go:
Patch Set #3, Line 91:
// must match at least one file or non-empty directory. The maximum supported file size is 2 GB.
Let's actually drop this part of the change and not document this limit. […]
Done
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
Rob Pike (Gerrit)
Attention is currently required from: Jay Conrod, Ian Lance Taylor, korzhao.
1 comment:
File src/cmd/compile/internal/staticdata/data.go:
Patch Set #4, Line 141:
return nil, 0, fmt.Errorf("file is larger than 2 GB")If you're going to fix this, fix it right. 2e9 should be a constant (and probably not 2e9 but instead something like 1<<31-1, if the comment is accurate) and then use that in the print statement. Something like
fmt.Errorf("%q has size %d; maximum allowed is %d", file, size, maxFileSize)
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
korzhao (Gerrit)
Attention is currently required from: Jay Conrod, Ian Lance Taylor, korzhao.
korzhao uploaded patch set #5 to this change.
embed: document the maximum file size supported
Fixes #47627
Change-Id: Ia1edfb6249863ab055fab68a35666bc2bdf21dcb
---
M src/cmd/compile/internal/staticdata/data.go
1 file changed, 6 insertions(+), 2 deletions(-)
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
korzhao (Gerrit)
Attention is currently required from: Jay Conrod, Ian Lance Taylor, Rob Pike.
1 comment:
File src/cmd/compile/internal/staticdata/data.go:
Patch Set #4, Line 141:
return nil, 0, fmt.Errorf("file is larger than 2 GB")
If you're going to fix this, fix it right. […]
Done
I found a similar Error, and i was consistent with it.
https://cs.opensource.google/go/go/+/master:src/cmd/internal/obj/objfile.go;l=371;drc=a7a17f0ca86d252dc1ef20b5852c352ade5f8610
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
Rob Pike (Gerrit)
Attention is currently required from: Jay Conrod, Ian Lance Taylor, korzhao.
1 comment:
File src/cmd/compile/internal/staticdata/data.go:
Patch Set #5, Line 145:
return nil, 0, fmt.Errorf("file too large (%d bytes > %d bytes)", size, cutoff)please put the file name here. the message is much more helpful if it says which file is the problem. the example you cite names the symbol, so it's consistent too.
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
korzhao (Gerrit)
Attention is currently required from: Jay Conrod, Ian Lance Taylor, Rob Pike.
1 comment:
File src/cmd/compile/internal/staticdata/data.go:
Patch Set #5, Line 145:
return nil, 0, fmt.Errorf("file too large (%d bytes > %d bytes)", size, cutoff)
please put the file name here. […]
I found that the file name could already be printed.
The format is:
./main.go:9:5: embed test.file: file too large (2147483648 bytes > 2000000000 bytes)https://cs.opensource.google/go/go/+/master:src/cmd/compile/internal/staticdata/embed.go;l=127
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
korzhao (Gerrit)
File src/cmd/compile/internal/staticdata/data.go:
Patch Set #5, Line 145:
return nil, 0, fmt.Errorf("file too large (%d bytes > %d bytes)", size, cutoff)
I found that the file name could already be printed. […]
Done
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
Attention is currently required from: Ian Lance Taylor, Rob Pike, korzhao.
Patch set 5:Run-TryBot +1
1 comment:
File src/cmd/compile/internal/staticdata/data.go:
Patch Set #5, Line 97:
const cutoff = int64(2e9)Let's name this maxFileSize as Rob suggested. cutoff is not a meaningful name.
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
korzhao (Gerrit)
Attention is currently required from: Ian Lance Taylor, Rob Pike, korzhao.
korzhao uploaded patch set #6 to this change.
embed: document the maximum file size supported
Fixes #47627
Change-Id: Ia1edfb6249863ab055fab68a35666bc2bdf21dcb
---
M src/cmd/compile/internal/staticdata/data.go
1 file changed, 6 insertions(+), 2 deletions(-)
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
korzhao (Gerrit)
Attention is currently required from: Jay Conrod, Ian Lance Taylor, Rob Pike.
1 comment:
File src/cmd/compile/internal/staticdata/data.go:
Patch Set #5, Line 97:
const cutoff = int64(2e9)Let's name this maxFileSize as Rob suggested. cutoff is not a meaningful name.
Done
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
Attention is currently required from: Ian Lance Taylor, Rob Pike, korzhao.
Patch set 6:Code-Review +2
1 comment:
Patchset:
Looks good, thanks!
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
Jay Conrod submitted this change.
embed: document the maximum file size supported
Fixes #47627
Change-Id: Ia1edfb6249863ab055fab68a35666bc2bdf21dcb
Reviewed-on: https://go-review.googlesource.com/c/go/+/341689
Reviewed-by: Jay Conrod <jayc...@google.com>
Trust: Michael Matloob <mat...@golang.org>
---
M src/cmd/compile/internal/staticdata/data.go
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/cmd/compile/internal/staticdata/data.go b/src/cmd/compile/internal/staticdata/data.go
index abb0bba..f25d8d8 100644
--- a/src/cmd/compile/internal/staticdata/data.go
+++ b/src/cmd/compile/internal/staticdata/data.go
@@ -92,6 +92,10 @@
return symdata
}
+// maxFileSize is the maximum file size permitted by the linker
+// (see issue #9862).
+const maxFileSize = int64(2e9)
+
// fileStringSym returns a symbol for the contents and the size of file.
// If readonly is true, the symbol shares storage with any literal string
// or other file with the same content and is placed in a read-only section.
@@ -133,12 +137,12 @@
}
return sym, size, nil
}
- if size > 2e9 {
+ if size > maxFileSize {
// ggloblsym takes an int32,
// and probably the rest of the toolchain
// can't handle such big symbols either.
// See golang.org/issue/9862.
- return nil, 0, fmt.Errorf("file too large")
+ return nil, 0, fmt.Errorf("file too large (%d bytes > %d bytes)", size, maxFileSize)
}
// File is too big to read and keep in memory.
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
Rob Pike (Gerrit)
Attention is currently required from: korzhao.
1 comment:
File src/cmd/compile/internal/staticdata/data.go:
Patch Set #7, Line 145:
return nil, 0, fmt.Errorf("file too large (%d bytes > %d bytes)", size, maxFileSize)you did not include the file name as i requested.
fmt.Errorf("%q too large (%d bytes > %d bytes)", file, size, maxFileSize)
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
File src/cmd/compile/internal/staticdata/data.go:
Patch Set #7, Line 145:
return nil, 0, fmt.Errorf("file too large (%d bytes > %d bytes)", size, maxFileSize)
you did not include the file name as i requested. […]
It looks like this error is wrapped in embed.go:127. So the output does include the file name:
./big.go:6:5: embed bigfile: file too large (3000000000 bytes > 2000000000 bytes)
Did you mean for the file name to be added here and the wrapping removed there?
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.
Rob Pike (Gerrit)
File src/cmd/compile/internal/staticdata/data.go:
Patch Set #7, Line 145:
return nil, 0, fmt.Errorf("file too large (%d bytes > %d bytes)", size, maxFileSize)
It looks like this error is wrapped in embed.go:127. So the output does include the file name: […]
as long as the file name returns to the user, i'm happy
To view, visit change 341689. To unsubscribe, or for help writing mail filters, visit settings.