sstate: Truncate PV in sstate filenames that are too long

sstate filenames are generated by concatenating a variety of bits of package metadata. Some of these parts could be long, which could cause the filename to be longer than the 255 character maximum for ext4.

So, let's try to detect this situation and truncate the PV part of the filename so that it will fit. If this happens, an ellipsis is added to make it clear that the version number is incomplete.

SSTATE_PKG needs to be consistent for all tasks so that the hash remains stable. This means that we need to make an assumption for the maximum length of the task name. In this implementation, the task name is limited to 27 characters.

This change also results in a sensible error message being emitted if the resulting filename is still too long.

Signed-off-by Mike Crowe <[email protected]>

meta/classes/sstate.bbclass +0 -0

index 3342c5ef50..6313b1c538 100644

meta/classes/sstate.bbclass +4 -4

@@ -323,7 +341,7 @@ def sstate_installpkg(ss, d):
 
     sstateinst = d.expand("${WORKDIR}/sstate-install-%s/" % ss['task'])
     sstatefetch = d.getVar('SSTATE_PKGNAME') + '_' + ss['task'] + ".tgz"
-    sstatepkg = d.getVar('SSTATE_PKG') + '_' + ss['task'] + ".tgz"
+    sstatepkg = sstate_path(ss['task'], d)
 
     if not os.path.exists(sstatepkg):
         pstaging_fetch(sstatefetch, d)

@@ -617,7 +635,7 @@ def sstate_package(ss, d):
     tmpdir = d.getVar('TMPDIR')
 
     sstatebuild = d.expand("${WORKDIR}/sstate-build-%s/" % ss['task'])
-    sstatepkg = d.getVar('SSTATE_PKG') + '_'+ ss['task'] + ".tgz"
+    sstatepkg = sstate_path(ss['task'], d)
     bb.utils.remove(sstatebuild, recurse=True)
     bb.utils.mkdirhier(sstatebuild)
     bb.utils.mkdirhier(os.path.dirname(sstatepkg))

@@ -1084,8 +1102,8 @@ python sstate_eventhandler() {
         if taskname in ["fetch", "unpack", "patch", "populate_lic", "preconfigure"] and swspec:
             d.setVar("SSTATE_PKGSPEC", "${SSTATE_SWSPEC}")
             d.setVar("SSTATE_EXTRAPATH", "")
-        sstatepkg = d.getVar('SSTATE_PKG')
-        bb.siggen.dump_this_task(sstatepkg + '_' + taskname + ".tgz" ".siginfo", d)
+        sstateinfo = sstate_path(taskname, d) + ".siginfo"
+        bb.siggen.dump_this_task(sstateinfo, d)
 }
 
 SSTATE_PRUNE_OBSOLETEWORKDIR ?= "1"

Quoted 23 lines · Mike Crowe

sstate filenames are generated by concatenating a variety of bits of package metadata. Some of these parts could be long, which could cause the filename to be longer than the 255 character maximum for ext4.

So, let's try to detect this situation and truncate the PV part of the filename so that it will fit. If this happens, an ellipsis is added to make it clear that the version number is incomplete.

SSTATE_PKG needs to be consistent for all tasks so that the hash remains stable. This means that we need to make an assumption for the maximum length of the task name. In this implementation, the task name is limited to 27 characters.

This change also results in a sensible error message being emitted if the resulting filename is still too long.

Signed-off-by Mike Crowe <[email protected]>

meta/classes/sstate.bbclass +0 -0

index 3342c5ef50..6313b1c538 100644

meta/classes/sstate.bbclass def generate_sstatefn(spec, hash, d): · line 13

Quoted 6 lines

meta/classes/sstate.bbclass +3 -0

@@ -8,6 +8,24 @@ def generate_sstatefn(spec, hash, d):
         hash = "INVALID"
     return hash[:2] + "/" + spec + hash
 
+def sstate_path(taskname, d):
+    max_filename_len = 245 # leave some room for ".siginfo"
+    max_addendum_len = 32 # '_' + taskname + '.tgz'

Since the task name is variable, is there really a 32 character limit here?

It may make sense to do:

# '_' + taskname + '.tgz', reserving a minimum of 32 for taskname max_addendum_len = len(taskname) + 5 if len(taskname) + 5 > 32 else 32

Always reserve a minimum of 32 for consistency, but if we go over account for it.

meta/classes/sstate.bbclass def generate_sstatefn(spec, hash, d): · line 19

Quoted 6 lines

meta/classes/sstate.bbclass +6 -0

+    sstate_prefix = d.getVar('SSTATE_PKG')
+    excess = len(os.path.basename(sstate_prefix)) - (max_filename_len - max_addendum_len)
+    if excess > 0:
+        pv = d.getVar('PV')
+        if len(pv) >= excess and len(pv) >= 3:
+            short_pv = d.getVar('PV')[:-excess-3] + '...'

Is truncating the PV enough? In a discussion on the bitbake list, I suggested possibly changing the order of the entries in the SSTATE_PKGSPEC to allow us to prune things prior to the hash w/o affecting the hash. Maybe this is simply not needed.. but it's a possibility if this proves to not be effective.

meta/classes/sstate.bbclass def generate_sstatefn(spec, hash, d): · line 31

Quoted 12 lines

meta/classes/sstate.bbclass +9 -0

+            d2 = d.createCopy()
+            d2.setVar('PV', short_pv)
+            sstate_prefix = d2.getVar('SSTATE_PKG')
+
+    sstatepkg = sstate_prefix + '_'+ taskname + ".tgz"
+    if len(os.path.basename(sstatepkg)) > max_filename_len:
+        bb.error('Failed to shorten sstate filename')
+    return sstatepkg
+
 SSTATE_PKGARCH    = "${PACKAGE_ARCH}"
 SSTATE_PKGSPEC    = "sstate:${PN}:${PACKAGE_ARCH}${TARGET_VENDOR}-${TARGET_OS}:${PV}:${PR}:${SSTATE_PKGARCH}:${SSTATE_VERSION}:"
 SSTATE_SWSPEC     = "sstate:${PN}::${PV}:${PR}::${SSTATE_VERSION}:"

There is something else I noticed.. "SSTATE_PKGNAME" defined as:

SSTATE_PKGNAME =
"${SSTATE_EXTRAPATH}${@generate_sstatefn(d.getVar('SSTATE_PKGSPEC'), d.getVar('BB_UNIHASH'), d)}"

From what I can tell, this really should be using the new sstate_path function in someway.

Would it make more sense to define SSTATE_PKGNAME in such a way that it always resulted in something "short" enough, and in the right format, that it would always work?

Adjusting or rewriting "generate_sstatefn" could still accomplish the PV change, but the max length of the string would need further shrinking to accommodate an unknown task length (which goes back to my previous comment). If the 32 default is long enough then that shouldn't be a problem -- and may also resolve my concerns that something outside of sstate class could try to use that various and without the new magic function get the wrong results.

Quoted 33 lines · Mark Hatle

Quoted 23 lines · Mike Crowe

sstate filenames are generated by concatenating a variety of bits of package metadata. Some of these parts could be long, which could cause the filename to be longer than the 255 character maximum for ext4.

So, let's try to detect this situation and truncate the PV part of the filename so that it will fit. If this happens, an ellipsis is added to make it clear that the version number is incomplete.

SSTATE_PKG needs to be consistent for all tasks so that the hash remains stable. This means that we need to make an assumption for the maximum length of the task name. In this implementation, the task name is limited to 27 characters.

This change also results in a sensible error message being emitted if the resulting filename is still too long.

Signed-off-by Mike Crowe <[email protected]>

meta/classes/sstate.bbclass +0 -0

index 3342c5ef50..6313b1c538 100644

meta/classes/sstate.bbclass def generate_sstatefn(spec, hash, d): · line 13

Quoted 6 lines

meta/classes/sstate.bbclass +3 -0

@@ -8,6 +8,24 @@ def generate_sstatefn(spec, hash, d):
         hash = "INVALID"
     return hash[:2] + "/" + spec + hash
 
+def sstate_path(taskname, d):
+    max_filename_len = 245 # leave some room for ".siginfo"
+    max_addendum_len = 32 # '_' + taskname + '.tgz'

Since the task name is variable, is there really a 32 character limit here?

Quoted 7 lines

It may make sense to do:

# '_' + taskname + '.tgz', reserving a minimum of 32 for taskname max_addendum_len = len(taskname) + 5 if len(taskname) + 5 > 32 else 32

Always reserve a minimum of 32 for consistency, but if we go over account for it.

I think that would just cause task hash mismatches (see third paragraph of commit message.)

It probably does make sense to detect such long task names in this situation and generate errors though.

Quoted 11 lines

meta/classes/sstate.bbclass def generate_sstatefn(spec, hash, d): · line 19

Quoted 6 lines

meta/classes/sstate.bbclass +6 -0

+    sstate_prefix = d.getVar('SSTATE_PKG')
+    excess = len(os.path.basename(sstate_prefix)) - (max_filename_len - max_addendum_len)
+    if excess > 0:
+        pv = d.getVar('PV')
+        if len(pv) >= excess and len(pv) >= 3:
+            short_pv = d.getVar('PV')[:-excess-3] + '...'

Is truncating the PV enough? In a discussion on the bitbake list, I suggested possibly changing the order of the entries in the SSTATE_PKGSPEC to allow us to prune things prior to the hash w/o affecting the hash. Maybe this is simply not needed.. but it's a possibility if this proves to not be effective.

Truncating PV solves the problem I was having. The other fields don't really tend to be very long, so there's less to be gained by shortening them.

Quoted 15 lines

meta/classes/sstate.bbclass def generate_sstatefn(spec, hash, d): · line 31

Quoted 12 lines

meta/classes/sstate.bbclass +9 -0

+            d2 = d.createCopy()
+            d2.setVar('PV', short_pv)
+            sstate_prefix = d2.getVar('SSTATE_PKG')
+
+    sstatepkg = sstate_prefix + '_'+ taskname + ".tgz"
+    if len(os.path.basename(sstatepkg)) > max_filename_len:
+        bb.error('Failed to shorten sstate filename')
+    return sstatepkg
+
 SSTATE_PKGARCH    = "${PACKAGE_ARCH}"
 SSTATE_PKGSPEC    = "sstate:${PN}:${PACKAGE_ARCH}${TARGET_VENDOR}-${TARGET_OS}:${PV}:${PR}:${SSTATE_PKGARCH}:${SSTATE_VERSION}:"
 SSTATE_SWSPEC     = "sstate:${PN}::${PV}:${PR}::${SSTATE_VERSION}:"

There is something else I noticed.. "SSTATE_PKGNAME" defined as:

Quoted 6 lines

SSTATE_PKGNAME =
"${SSTATE_EXTRAPATH}${@generate_sstatefn(d.getVar('SSTATE_PKGSPEC'), d.getVar('BB_UNIHASH'), d)}"

From what I can tell, this really should be using the new sstate_path function in someway.

Hmm, you're right. I can't have covered that in my testing. :(

Quoted 3 lines

Would it make more sense to define SSTATE_PKGNAME in such a way that it always resulted in something "short" enough, and in the right format, that it would always work?

I'd considered doing it that way. If SSTATE_OKGSPEC contained ${SSTATE_PV} and that either had the value of ${PV} or the a truncated version of ${PV} then the rest of the file could remain the same. However, truncating PV without access to the rest of the spec would mean just picking some arbitrary maximum PV length which is likely to be more conservative than necessary.

Quoted 6 lines

Adjusting or rewriting "generate_sstatefn" could still accomplish the PV change, but the max length of the string would need further shrinking to accommodate an unknown task length (which goes back to my previous comment). If the 32 default is long enough then that shouldn't be a problem -- and may also resolve my concerns that something outside of sstate class could try to use that various and without the new magic function get the wrong results.

I wonder whether I can get away with applying per-task PV truncation in generate_sstatefn without causing hash mismatches? That's worth a try.

Thanks for your comments. They've been very helpful.

Mike.

Richard mentioned in the bitbake-devel thread that there may be external tools using some of the components. I'm not sure how to even identify what those tools are at this time, but a new sstate version entry may be enough to start to deal with them.

Quoted 55 lines · Mike Crowe

Quoted 33 lines · Mark Hatle

Quoted 23 lines · Mike Crowe

sstate filenames are generated by concatenating a variety of bits of package metadata. Some of these parts could be long, which could cause the filename to be longer than the 255 character maximum for ext4.

So, let's try to detect this situation and truncate the PV part of the filename so that it will fit. If this happens, an ellipsis is added to make it clear that the version number is incomplete.

SSTATE_PKG needs to be consistent for all tasks so that the hash remains stable. This means that we need to make an assumption for the maximum length of the task name. In this implementation, the task name is limited to 27 characters.

This change also results in a sensible error message being emitted if the resulting filename is still too long.

Signed-off-by Mike Crowe <[email protected]>

meta/classes/sstate.bbclass +0 -0

index 3342c5ef50..6313b1c538 100644

meta/classes/sstate.bbclass def generate_sstatefn(spec, hash, d): · line 13

Quoted 6 lines

meta/classes/sstate.bbclass +3 -0

@@ -8,6 +8,24 @@ def generate_sstatefn(spec, hash, d):
         hash = "INVALID"
     return hash[:2] + "/" + spec + hash
 
+def sstate_path(taskname, d):
+    max_filename_len = 245 # leave some room for ".siginfo"
+    max_addendum_len = 32 # '_' + taskname + '.tgz'

Since the task name is variable, is there really a 32 character limit here?

Quoted 7 lines

It may make sense to do:

# '_' + taskname + '.tgz', reserving a minimum of 32 for taskname max_addendum_len = len(taskname) + 5 if len(taskname) + 5 > 32 else 32

Always reserve a minimum of 32 for consistency, but if we go over account for it.

I think that would just cause task hash mismatches (see third paragraph of commit message.)

It probably does make sense to detect such long task names in this situation and generate errors though.

Quoted 7 lines

Quoted 6 lines

meta/classes/sstate.bbclass +6 -0

+    sstate_prefix = d.getVar('SSTATE_PKG')
+    excess = len(os.path.basename(sstate_prefix)) - (max_filename_len - max_addendum_len)
+    if excess > 0:
+        pv = d.getVar('PV')
+        if len(pv) >= excess and len(pv) >= 3:
+            short_pv = d.getVar('PV')[:-excess-3] + '...'

Quoted 8 lines

Quoted 4 lines

Is truncating the PV enough? In a discussion on the bitbake list, I suggested possibly changing the order of the entries in the SSTATE_PKGSPEC to allow us to prune things prior to the hash w/o affecting the hash. Maybe this is simply not needed.. but it's a possibility if this proves to not be effective.

Truncating PV solves the problem I was having. The other fields don't really tend to be very long, so there's less to be gained by shortening them.

Quoted 42 lines

Quoted 15 lines

meta/classes/sstate.bbclass def generate_sstatefn(spec, hash, d): · line 31

Quoted 12 lines

meta/classes/sstate.bbclass +9 -0

+            d2 = d.createCopy()
+            d2.setVar('PV', short_pv)
+            sstate_prefix = d2.getVar('SSTATE_PKG')
+
+    sstatepkg = sstate_prefix + '_'+ taskname + ".tgz"
+    if len(os.path.basename(sstatepkg)) > max_filename_len:
+        bb.error('Failed to shorten sstate filename')
+    return sstatepkg
+
 SSTATE_PKGARCH    = "${PACKAGE_ARCH}"
 SSTATE_PKGSPEC    = "sstate:${PN}:${PACKAGE_ARCH}${TARGET_VENDOR}-${TARGET_OS}:${PV}:${PR}:${SSTATE_PKGARCH}:${SSTATE_VERSION}:"
 SSTATE_SWSPEC     = "sstate:${PN}::${PV}:${PR}::${SSTATE_VERSION}:"

There is something else I noticed.. "SSTATE_PKGNAME" defined as:

Quoted 6 lines

SSTATE_PKGNAME =
"${SSTATE_EXTRAPATH}${@generate_sstatefn(d.getVar('SSTATE_PKGSPEC'), d.getVar('BB_UNIHASH'), d)}"

From what I can tell, this really should be using the new sstate_path function in someway.

Hmm, you're right. I can't have covered that in my testing. :(

Quoted 3 lines

Would it make more sense to define SSTATE_PKGNAME in such a way that it always resulted in something "short" enough, and in the right format, that it would always work?

I'd considered doing it that way. If SSTATE_OKGSPEC contained ${SSTATE_PV} and that either had the value of ${PV} or the a truncated version of ${PV} then the rest of the file could remain the same. However, truncating PV without access to the rest of the spec would mean just picking some arbitrary maximum PV length which is likely to be more conservative than necessary.

Quoted 6 lines

Adjusting or rewriting "generate_sstatefn" could still accomplish the PV change, but the max length of the string would need further shrinking to accommodate an unknown task length (which goes back to my previous comment). If the 32 default is long enough then that shouldn't be a problem -- and may also resolve my concerns that something outside of sstate class could try to use that various and without the new magic function get the wrong results.

Quoted 2 lines

I wonder whether I can get away with applying per-task PV truncation in generate_sstatefn without causing hash mismatches? That's worth a try.

The only cause of a task hash mismatch (actual hash, not filename) would be in SSTATE_PKGNAME is part of the hash itself. I'd contend if it is, then we should exclude it.