Hack permission fix until core is updated

[oscarlevin]

No description provided.

[StevenClontz]

can you merge this with #657 to see if tests pass? (or re-implement my test here)

[oscarlevin]

Okay, I see what's happening. You put the test on the non-cli calls. Do you (@StevenClontz) think we need to patch this now, or can that wait for fixes to core?

[oscarlevin]

Turns out it was easy to move into the project's build command, so I just did that.

[oscarlevin]

Alright @StevenClontz. Can you mark your review as approved so we can merge?

[StevenClontz]

Turns out it was easy to move into the project's build command, so I just did that.

Thanks. IMO the lighter-weight CLI is, the better. It's somewhat interesting that pretext new is implemented purely in the CLI, but it's unclear to me how it would work otherwise: Project() knows only the contents of project.ptx; the actual project itself is all on the disk.

[bnmnetp]

I think this PR (or maybe the underlying core problem??) is the cause of recent unpleasantness on Runestone Academy. Suddenly and mysteriously the permissions on the output folder for runestone build books namely published/document-id have lost the execute bit setting for "group" and "other". Because the servers do not run as root, the lack of the x bit doesn't let them see some of the files they need in order to serve pages properly.

I don't think it is good practice to override a users umask settings, or even to mess around with permissions at all.

[oscarlevin]

Can you say more @bnmnetp? I agree that I don't want to be in the business of permission management. The problem is that:

• python's command to create a temporary directory always sets the permissions of the temp directory to 700.
• Now that we are using shutil.copy() to move the built files from the temp directory to the output folder, those permissions are preserved.

The hack that this PR uses assumes that the output folder's parent folder has the permissions that should be on the output folder, and just coerces those after moving the built files.

The proposed fix to core would set the permissions to the temp directories created to 755, and then we can revert the hack. But I'm not sure that would help your situation. What do you think?

[StevenClontz]

To make sure I understand: you want published/document-id to have permissions 777.

For clarity, somewhere (can't find the thread) we found that a core Python routine sets the temporary directory permission at 700, so you'll need to take that up with Python maintainers. Oscar implemented

pretext-cli/pretext/project/__init__.py

Line 657 in 745602a

# Temporary hack to ensure the correct permissions are set on the output directory.

which I believe forces permissions of a build directory to match the project directory. Does your project directory have permissions 777?

As I recall, we have a better fix that needs to be merged into PreTeXtBook/pretext - I'll ping that now...

[bnmnetp]

I'm not asking for 777 I'm asking for at least 755 or 775.

Perhaps using shutil.copy to copy from a tmp directory is not the best solution. I don't think I would get very far with the Python maintainers.

I think I may not have had the version with Oscar's hack in it. because either remembering what the permissions on the output directory were and then restoring them after the copy, or using the permissions of the parent is also pretty reasonable.

In any case I have not seen any of the output folders set to 700.

[StevenClontz]

I think I may not have had the version with Oscar's hack in it.

Sounds likely - we've since added tests to our suite to ensure that the lowest permissions our new projects and builds should be are 755:

pretext-cli/tests/test_cli.py

Line 55 in 745602a

assert (tmp_path / "new-pretext-project").stat().st_mode % 0o1000 >= 0o755

pretext-cli/tests/test_project.py

Line 308 in 745602a

assert (prj_path / "output" / "web").stat().st_mode % 0o1000 >= 0o755