Skip to content

Create missing symbolic links in a course templates directory when upgrading. #2757

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

drgrice1
Copy link
Member

This finishes @Alex-Jordan's work in #2750. It checks for missing symbolic links in a course's templates directory when checking if courses need upgrading, and creates the missing links when upgrading the course. If a symbolic link points to an incorrect location (as defined in the course environment), then the upgrade process attempts to delete the symbolic link and recreate it pointing to the correct location.

Note that the check for permissions of the symbolic links has been removed. For symbolic links there are two permissions in consideration. Those of the link itself, and those of the link target. The permissions of the link itself cannot be changed. Even the linux command line chmod utility cannot do this (contrary to what Google AI wants to tell you, there is no -h option for chmod). Furthermore, linux completely ignores symbolic link permissions, and doesn't use them at all. So the only permissions that really matter are those of the link target which linux does use. However, the directories that are the target of these links do not belong to a particular course. They are system directories (or part of the webwork2 installation), and setting the correct permissions on those is part of the installation of webwork2. It should not be part of a course upgrade process. Including these permissions in the upgrade report when not correct would actually mean that every course would show that it has the incorrect permission, or no courses would. Upgrading would never fix this, because the permissions for these directories cannot be changed from the user interface in any case. What could be done here is to make another admin page that does a system status check, and lets the admin user know what directory permissions in the webwork installation are not correct what needs to be fixed. Of course the setfilepermissions script can be used to fix these permissions (it already handles most of the directories in question).

@Alex-Jordan: Hopefully you are okay with me opening a new pull request instead of making a pull request to your branch. I thought this would be easier for me to maintain, since you said you didn't have time now to finish that pull request.

Alex-Jordan and others added 2 commits June 24, 2025 20:34
…grading.

This finishes @Alex-Jordan's work in openwebwork#2750, and checks for missing
symbolic links in a course's templates directory when checking if
courses need upgrading, and creates the missing links when upgrading the
course.  If a symbolic link points to an incorrect location (as defined
in the course environment), then the upgrade process attempts to delete
the symbolic link and recreate it pointing to the correct location.

Note that the check for permissions of the symbolic links has been
removed.  For symbolic links there are two permissions in consideration.
Those of the link itself, and those of the link target.  The permissions
of the link itself cannot be changed.  Even the linux command line
`chmod` utility cannot do this (contrary to what Google AI wants to tell
you, there is no `-h` option for `chmod`).  Furthermore, linux
completely ignores symbolic link permissions, and doesn't use them at
all.  So the only permissions that really matter are those of the link
target which linux does use.  However, the directories that are the
target of these links do not belong to a particular course.  They are
system directories (or part of the webwork2 installation), and setting
the correct permissions on those is part of the installation of
webwork2. It should not be part of a course upgrade process. Including
these permissions in the upgrade report when not correct would actually
mean that every course would show that it has the incorrect permission,
or no courses would. Upgrading would never fix this, because the
permissions for these directories cannot be changed from the user
interface in any case.  What could be done here is to make another admin
page that does a system status check, and lets the admin user know what
directory permissions in the webwork installation are not correct what
needs to be fixed. Of course the `setfilepermissions` script can be used
to fix these permissions (it already handles most of the directories in
question).
@Alex-Jordan
Copy link
Contributor

Thanks for picking this up. I will test it soon, perhaps this evening.

Copy link
Contributor

@Alex-Jordan Alex-Jordan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This worked for me to create missing links and change the target of a link that was bad.

One thing that might pair with this PR though. When using the Archive Courses tool on a course with bad directories or links, you get the options to "Attempt to upgrade directories or links".

But when you use Rename Course, you don't get that option. You are told you must manually repair the directories and/or links. Is there any reason not to have the option here too?

Copy link
Member

@pstaabp pstaabp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well. I agree with @Alex-Jordan if it's not a heavy lift to handle the rename course to fix the links as well.

Also fix the logic both here and when archiving a course for the
possible combinations of the directories, links, and database needing
repair.  With the previous logic if the directories were good, but the
database tables and links both needed repair, then the button said
"Attempt to upgrade directories and links".  However, in actuality if
that button were used, the server would both repair links and repair
database tables.  Now, anytime the database tables need repair the
button says "Upgrade Course Tables" regardless of the state of the
directories and links. This is not ideal since directories or links
might also be repaired in this case, but the important thing to show is
the database repair which is more important than directories and vastly
more important than links.
@drgrice1
Copy link
Member Author

Now there is an option to fix directories and links when renaming a course.

Also the logic was fixed both for renaming and archiving a course for the possible combinations of the directories, links, and database needing repair. With the previous logic if the directories were good, but the database tables and links both needed repair, then the button said "Attempt to upgrade directories and links". However, in actuality if that button were used, the server would both repair links and repair database tables. Now, anytime the database tables need repair the button says "Upgrade Course Tables" regardless of the state of the directories and links. This is not ideal since directories or links might also be repaired in this case, but the important thing to show is the database repair which is more important than directories and vastly more important than links.

Copy link
Contributor

@somiaj somiaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

One thing I notice is I prefer relative symlinks vs absolute, but this is just due to my setup, for instance I would prefer ../../../libraries/webwork-open-problem-library/OpenProblemLibrary/ over /opt/webwork/libraries/webwork-open-problem-library/OpenProblemLibrary/. I did test that the relative symlinks do get validated correctly, so this is just a very minor thing when creating the new links.

Though I guess this just falls into a more complicated setup and I can manually create links for my use case. Also any bad links can be fixed when moving a course to a different server with these new tools. Just sharing a random thought mostly.

@drgrice1
Copy link
Member Author

With using the directories from the course environment it is not possible to use relative paths for the links.

Actually, not using relative paths for the symbolic links is better than the current links in the model course anyway. Perhaps you like relative links, but using absolute paths solves a problem. That is that theoretically the location of the courses directory is configurable, as is the location of the OPL. So if someone uses a non-standard location for the courses directory or for the OPL, then the links in the model course won't work. However, the links created by this method will. It seems that what we really should do is remove the links from the model course, and when a course is created the links should be created from the configuration. Perhaps when unarchiving a course the links could even be created/fixed at that time. Then there would be no need for handling this in the course update, and the links would always be there and work.

@Alex-Jordan Alex-Jordan merged commit 0ab4513 into openwebwork:WeBWorK-2.20 Jun 27, 2025
2 checks passed
@drgrice1 drgrice1 deleted the update-links-completion branch June 27, 2025 12:15
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.

4 participants