Remove -Xmax-classfile-length; hard-code to 240 #7497
Conversation
| 2 * (settings.maxClassfileName.value - maxSuffixLength - 2*marker.length - 32) | ||
| ) | ||
| final val marker = "$$$$" | ||
| final val MaxSuffixLength = 7 // "$.class".length + 1 // potential module class suffix and file extension |
There was a problem hiding this comment.
actually, this is different (8 -> 7). I can't tell why the +1 was in maxSuffixLength...
|
do we need to deprecate-then-remove? |
|
Your call. It can get deprecated in 2.12.9, right? |
yes but it's a bit weird since very likely 2.12.9 won't come out until after 2.13.0-RC1. and there's no particular rush, right? |
|
I'd say the flag can go without depracation, given how |
|
the only recent discussion in this area I can find is at #5088 |
|
also I closed scala/bug#3623, we'll see if that produces any reaction |
|
For our use cases changing the behavior is fine (and I would be happy to have this in 2.12.x) . In general it breaks binary compatability though doesn't it (e.g. serialisation). It seems that it was already broken and this is stability though so it seems better that we had before |
|
Changing it breaks binary/serialisation compat, yes. This is 2.13 so neither of those are guaranteed anyways. If we did the other (more complicated) solution of pickling flattened names it would allow artifacts produced with one value to be used to compile sources even with a different value; this might be helpful to e.g. the ecryptfs people. |
|
People on #5088 seem to report that |
|
I'd go for 240 then if that helps running in docker (https://stackoverflow.com/a/42840554/248998) |
|
Though note that ecryptfs developers recommend using a limit of 140 characters (https://unix.stackexchange.com/a/32834) and I think that on Ubuntu, /home is encrypted with ecryptfs by default. |
|
I wonder how common ecryptfs is. Did we ever get reports of people being affected by this? It also seems they removed ecryptfs from ubuntu in 18.04 (https://www.linuxuprising.com/2018/04/how-to-encrypt-home-folder-in-ubuntu.html). Is full-disk encryption affected? |
|
I think limiting only names of private classes will not break compatibility. |
This option allowed users to limit the size of classfile names to fewer than 255 (previously the default) characters. However, this means that classfiles made from a compiler with the option are not compatible with classfiles made without it (in either direction), since the compiler will flatten names of classes with the current value of the setting, not whatever the class was compiled with. So, remove it.
adriaanm
force-pushed
the
kill/Xmax-classfile-name
branch
from
db2148c to
a8fa7e6
Compare
adriaanm
changed the title
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
I took a look at how popular ecryptfs might be, and I conclude we shouldn't worry too much. https://askubuntu.com/questions/1029249/how-to-encrypt-home-on-ubuntu-18-04 |
SethTisue
changed the title
8 participants
This option allowed users to limit the size of classfile names
to fewer than 255 (previously the default) characters.
However, this means that classfiles made from a compiler with the
option are not compatible with classfiles made without it
(in either direction), since the compiler will flatten names of
classes with the current value of the setting, not whatever the
class was compiled with.
So, remove it and default to 240 (the limit in docker).
See also scala/bug#8199