Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#24826 closed Bug (fixed)

file_storage.tests.FileFieldStorageTests.test_extended_length_storage fails when AUFS is in use

Reported by: Raphaël Hertzog Owned by: nobody
Component: File uploads/storage Version: 1.8
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

It's quite common to build Debian packages with an AUFS overlay on top of a clean build environment. However such an overlay imposes a slightly lower maximum length for filenames. The maximum is 242 according to http://aufs.sourceforge.net/aufs3/man.html

Due to this, the test file_storage.tests.FileFieldStorageTests.test_extended_length_storage fails and the package build fails. This is rather unfortunate. I'm not quite sure what the point of the test is so I'm not sure what's the best solution:

  • always use 238 "a" instead of 251
  • detect AUFS usage and use 238 "a" in that context
  • detect AUFS usage and skip the test in that context
  • do some tests to detect the true max filename and then pick the appropriate number of "a"

Only the last one seems really future proof to me because there are probably multiple contexts in which the maximum length of the filenames might be reduced (remote FS, other kinds of overlay FS, etc.).

======================================================================
ERROR: test_extended_length_storage (file_storage.tests.FileFieldStorageTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/«PKGBUILDDIR»/tests/file_storage/tests.py", line 539, in test_extended_length_storage
    obj.extended_length.save('%s.txt' % filename, ContentFile('Same Content'))
  File "/«PKGBUILDDIR»/django/db/models/fields/files.py", line 94, in save
    self.name = self.storage.save(name, content, max_length=self.field.max_length)
  File "/«PKGBUILDDIR»/django/core/files/storage.py", line 64, in save
    name = self._save(name, content)
  File "/«PKGBUILDDIR»/django/core/files/storage.py", line 249, in _save
    fd = os.open(full_path, flags, 0o666)
OSError: [Errno 36] File name too long: '/tmp/django_GfeLeZ/tmpQaF0eu/tests/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.txt'

Attachments (1)

fix-test-extended-length-storage.patch (2.4 KB) - added by Raphaël Hertzog 5 years ago.
Tested patch

Download all attachments as: .zip

Change History (9)

comment:1 Changed 5 years ago by Claude Paroz

Triage Stage: UnreviewedAccepted

comment:2 Changed 5 years ago by Raphaël Hertzog

It would be acceptable if it worked... :-)

I still have the failure with the patch. I'm not sure what is the root cause of the problem. I tried os.pathconf() manually in a build chroot and it gives the correct result when I give it a path under an AUFS mount (which includes /tmp).

However the build directory itself (where the Django source tree resides) is not an AUFS mount and there os.pathconf() returns 255.

I added some debug statements in the MAX_FILENAME_LENGTH property but it seems it's never called. Does the inherited variable take precedence for some reason?

comment:3 Changed 5 years ago by Raphaël Hertzog

Or rather it's called once rather early, possibly at a point where "self.location" doesn't exist yet... so that os.pathconf() throws an OSError "No such file or directory" which gets caught by the try clause and thus fallbacks to the 255 max length.

Changed 5 years ago by Raphaël Hertzog

Tested patch

comment:4 Changed 5 years ago by Raphaël Hertzog

The above patch is a modification of your patch to find the first parent directory that actually exists and use that with os.pathconf(). This patch is tested, it works for me.

comment:6 Changed 5 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:7 Changed 5 years ago by Claude Paroz <claude@…>

Resolution: fixed
Status: newclosed

In 170f7115:

Fixed #24826 -- Accounted for filesystem-dependent filename max length

Thanks Raphaël Hertzog for the report and help on the patch, and Tim Graham
for the review.

comment:8 Changed 5 years ago by Tim Graham <timograham@…>

In 0bfe322b:

[1.8.x] Fixed #24826 -- Accounted for filesystem-dependent filename max length

Thanks Raphaël Hertzog for the report and help on the patch, and Tim Graham
for the review.

Backport of 170f7115bbae45f26ca8078e749dfe67445a57ea from master

Note: See TracTickets for help on using tickets.
Back to Top