Skip to content

Determine the unarchive source course ID from the archive contents, not its filename#3043

Open
drdrew42 wants to merge 1 commit into
openwebwork:WeBWorK-2.21from
drdrew42:bugfix/unarchive-course-id-from-archive
Open

Determine the unarchive source course ID from the archive contents, not its filename#3043
drdrew42 wants to merge 1 commit into
openwebwork:WeBWorK-2.21from
drdrew42:bugfix/unarchive-course-id-from-archive

Conversation

@drdrew42

@drdrew42 drdrew42 commented Jul 7, 2026

Copy link
Copy Markdown
Member

Description:

Problem

Unarchiving a course whose .tar.gz has been renamed restores the course files but
not the database. The unarchive still reports success and leaves a new, database-less
course in place; the indication that something went wrong is a WeBWorK warning shown on
the admin page (and logged):

  course 'X' has no database dump in its data directory
  (checked for .../X/DATA/mysqldump). database tables will not be restored.

That warning is itself misleading: it implies the archive contains no database dump,
when in fact the dump is present — stored under the course's original name, which the
unarchive never looks at (see Cause). So an operator sees a course that seems to import
"with a warning" but is actually missing every user, set, and answer, plus a stray
files-only course directory that then blocks a corrected re-import with
"Cannot overwrite existing course."

Cause

do_unarchive_course derives the source course ID from the archive's filename
(oldCourseID => $unarchive_courseID =~ s/\.tar\.gz$//r), but Archive::Tar->extract()
writes files to the directory name stored inside the archive (the course's name at
archive time). These agree only until someone renames the file. When they diverge:

  • extract() writes course files under the archived name;
  • the database-dump lookup, the conflicting-course guard (_unarchiveCourse_move_away),
    and the post-restore rename all use the filename-derived name.

So the DB dump is sought in a directory that extraction never created → skipped.

Fix

Take the source course ID from the archive's single top-level directory, in
unarchiveCourse (the one chokepoint shared by the admin UI, WebworkWebservice, and CLI
callers). die if the archive doesn't contain exactly one top-level directory. The
caller's newCourseID still controls the target name, so renaming on import continues to
work via the New course ID field rather than by renaming the file.

Testing

  1. Archive a course Foo from the admin course (produces Foo.tar.gz).
  2. Rename/upload the archive as Bar.tar.gz.
  3. Unarchive it with new course ID Bar.
    • Before: files extract under Foo/; the unarchive reports success but shows the
      warning "…database tables will not be restored"; the new Bar course has no database,
      and a files-only Foo directory is left behind.
    • After: source ID read as Foo, DB dump found and restored, course renamed to Bar.
  4. Regression: a normally-named archive (Foo.tar.gzFoo) unarchives exactly as before.

unarchiveCourse() derived the source course ID from the caller-supplied
oldCourseID, which the admin controller computes from the archive's *filename*.
Archive::Tar->extract(), however, always writes files to the directory name
stored *inside* the archive (the course's name at archive time). These agree
only until someone renames the .tar.gz.

When they diverge (e.g. an archive renamed before import), the course files
extract under the archived name while the database-dump lookup, the
conflicting-course guard (_unarchiveCourse_move_away), and the post-restore
rename all use the filename-derived name. The result is a course whose files
restore correctly but whose database is not restored -- reported only as a
warning while the unarchive otherwise "succeeds":

  course 'X' has no database dump in its data directory
  (checked for .../X/DATA/mysqldump). database tables will not be restored.

Fix at the single chokepoint in unarchiveCourse() so every caller is covered:
open the archive first and take the source course ID from its sole top-level
directory, dying if the archive does not contain exactly one. The caller's
newCourseID still controls the target name, so renaming on import continues to
work via the "New course ID" field rather than by renaming the file.

The oldCourseID option is now unused, so drop it from the sole caller
(do_unarchive_course) and from the POD.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@drdrew42 drdrew42 force-pushed the bugfix/unarchive-course-id-from-archive branch from b4fadf7 to 5093faf Compare July 7, 2026 21:24
@Alex-Jordan

Copy link
Copy Markdown
Contributor

What is the use case for renaming a course archive file? And shouldn't that be discouraged? It would be confusing to unarchive alex.tar.gz and get a course named drew. I'm wondering if this is the right way to address the issue, or if there is something else, like error out early if the file name does not match the course ID inside it. Or give users a choice. Or a utility that renames course archive files including all the relevant internal bits.

@drdrew42

drdrew42 commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

There isn't a "use case" for it, so much as it's an issue that's surfaced here and there (for me).

The justification is that it shouldn't matter what the file is called, as the archive contains an "original" course name already. I get your point that you don't want to unarchive "alex" and get "drew" -- but at present, an archive named "alex" that holds a course named "drew" won't unarchive successfully in the first place.

Moreover, there always remains the option to specify the course name after un-archival -- which still applies.

I don't really see this as a major issue -- I mean, we've had this failure in the code for how long now, and no one's addressed it? I'm just putting this PR out as a result of one of my WW admins running into this issue. IMO, unarchive shouldn't fail just because the filename is "wrong"...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants