Skip to content
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

Fix the check_latex script. #2545

Merged

Conversation

drgrice1
Copy link
Member

The addition of the requirement of PG files for hardcopy (namely the pg.sty file), means that the check_latex script needs to know where the pg directory is. So find that the same way the webwork2 app does.

This fixes issue #2543.

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.

On my develop server check_latex runs successfully whether I am on the develop branch or this PR branch. I am not sure why. Approving this, but I don't feel like I tested it since I had no change in behavior.

@somiaj
Copy link
Contributor

somiaj commented Sep 2, 2024

I am also not able to reproduce the error, even if I unset PG_ROOT environment variable in develop.

I did notice that the output of the script changed, before it would only output Compilation Success!, now it outputs the line /opt/webwork/pg/assets/tex (or wherever pg is located). Any reason to have that line outputted without any context?

On a side note the script did fail with bidi.sty missing if I didn't have texlive-lang-arabic. I didn't notice any issues with not having this, so is this really required or optional? Anyways I'm okay either way, just something I noticed.

@somiaj
Copy link
Contributor

somiaj commented Sep 2, 2024

note, just because I don't think I was clear, it still outputs Compilation Success!, it just also outputs the line of the pg tex directory with this patch applied.

@drgrice1 drgrice1 force-pushed the bugfix/check_latex_needs_pg branch from 4cac5c0 to 27ace61 Compare September 2, 2024 14:19
@drgrice1
Copy link
Member Author

drgrice1 commented Sep 2, 2024

The output of the pg directory was a debugging line that I added and forgot to remove. It has been removed now.

@drgrice1
Copy link
Member Author

drgrice1 commented Sep 2, 2024

If the PG_ROOT environment variable is not set, then I don't know how the check_latex script in develop could possibly succeed.

I also have noticed the bidi package issue. That should not be in the check_latex_exam.tex file. The only hardcopy themes that use the bidi package are the Hebrew themes. So the only time you need the package is if you are using those themes. Since those themes are not enabled by default, we shouldn't check for it. Perhaps we could check to see if those themes are enabled for the site, and only check that for that package in that case.

The addition of the requirement of PG files for hardcopy (namely the
`pg.sty` file), means that the `check_latex` script needs to know where
the pg directory is.  So find that the same way the webwork2 app does.

This fixes issue openwebwork#2543.
@drgrice1 drgrice1 force-pushed the bugfix/check_latex_needs_pg branch from 27ace61 to 42cfb80 Compare September 2, 2024 14:40
@somiaj
Copy link
Contributor

somiaj commented Sep 2, 2024

I still cannot get check_latex to fail in main/develop. I even added your debug line to check_latex in develop and I'm getting that $pg{directories}{assetsTex} is set to the pg directory correctly from the setting in defaults.conf. The only way I can get check_latex to fail is to modify defaults.conf and make $pg{directories}{assetsTex} point to a wrong location.

Further I even tested setting $PG_ROOT=/bad/dir (an invalid location), and it has no effect, so in my testing $PG_ROOT is not being used by check_latex. What could be different with my system that $PG_ROOT isn't being used at all here?

@somiaj
Copy link
Contributor

somiaj commented Sep 2, 2024

Okay, I tracked it down, in site.conf I have $pg_dir = /opt/webwork/pg, this is no longer is site.conf.dist, so for users with older site.conf that haven't removed this setting, things can still find the pg dir and ignore PG_ROOT altogether. If I remove this setting from site.conf, I can reproduce the error if PG_ROOT is unset.

Furthermore, the patch fixes the issue.

@drgrice1
Copy link
Member Author

drgrice1 commented Sep 2, 2024

Ahh, that makes sense. If you still have the old setting in site.conf, then the course environment would get it from there.

@Alex-Jordan Alex-Jordan merged commit e2b849c into openwebwork:develop Sep 3, 2024
2 checks passed
Alex-Jordan added a commit that referenced this pull request Sep 3, 2024
…tfix

Fix the `check_latex` script. (hotfix of #2545)
@drgrice1 drgrice1 deleted the bugfix/check_latex_needs_pg branch September 3, 2024 22:30
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.

3 participants