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 permissions issues for pretext new and pretext build [html format]. #652

Closed
StevenClontz opened this issue Dec 21, 2023 · 16 comments · Fixed by #658
Closed

fix permissions issues for pretext new and pretext build [html format]. #652

StevenClontz opened this issue Dec 21, 2023 · 16 comments · Fixed by #658

Comments

@StevenClontz
Copy link
Member

https://groups.google.com/g/pretext-support/c/4YGa5UnTBnU

@StevenClontz
Copy link
Member Author

I did some troubleshooting in that thread...

The output/web problem is either an issue with the core PreTeXt python module, or perhaps how we're importing it somehow?

>>> pretext.core.html('/workspaces/effective-funicular/new-pretext-project/source/main.ptx', '/workspaces/effective-funicular/new-pretext-project/publication/p
ublication.ptx', {}, None, "html", extra_xsl=None, out_file=None, dest_dir='/workspaces/effective-funicular/new-pretext-project/output/web')
@StevenClontz ➜ /workspaces/effective-funicular/new-pretext-project (main) $ ls -la output/
total 12
drwxr-xrw-+ 3 codespace codespace 4096 Dec 21 17:00 .
drwxr-xr-x+ 8 codespace codespace 4096 Dec 21 17:00 ..
drwx------+ 5 codespace codespace 4096 Dec 21 17:00 web

It is not an issue with how CLI imports pretext.py:

>>> pretext.pretext.html('/workspaces/effective-funicular/new-pretext-project/source/main.ptx', '/workspaces/effective-funicular/new-pretext-project/publication/publication.ptx', {}, None, "html", extra_xsl=None, out_file=None, dest_dir='/workspaces/effective-funicular/new-pretext-project/output/web')
@StevenClontz ➜ /workspaces/effective-funicular/pretext (master) $ ls -la /workspaces/effective-funicular/new-pretext-project/output/
total 12
drwxr-xrw-+ 3 codespace codespace 4096 Dec 21 17:08 .
drwxr-xr-x+ 8 codespace codespace 4096 Dec 21 17:08 ..
drwx------+ 4 codespace codespace 4096 Dec 21 17:08 web

(to be clear, I git cloned the latest PreTeXtBook/pretext and imported pretext.pretext directly from Rob's repository in the last post)

@StevenClontz
Copy link
Member Author

I'm stepping away from this for a bit, but a theory: I seem to recall that, perhaps, both our provisioning of a new pretext project and Rob's building of an HTML target happen in a temporary directory. Perhaps the permissions of this temporary directory are 700, and we need to manually fix permissions when we copy things over?

@oscarlevin
Copy link
Member

Waiting for my codespace to spin up, but perhaps this has something to do with shutil.copytree using copy2() by default which is supposed to preserve permissions (and that when we create files in the temp directory, they get set to be more restrictive). There is a way to change the copy method. https://docs.python.org/3/library/shutil.html#shutil.copytree

@StevenClontz
Copy link
Member Author

It looks like this likewise is the case at https://github.com/PreTeXtBook/pretext/blob/master/pretext/pretext.py#L3326

@StevenClontz
Copy link
Member Author

Another solution: rather than building directly in to a temporary directory, we/core can create a directory inside the temporary directory (which I think will have the right permissions), build things there, and copy that.

@oscarlevin
Copy link
Member

I guess it makes sense that this is new, since it wasn't that long ago that we convinced @rbeezer to switch to shutil.copy() requiring the bump to 3.8

@oscarlevin
Copy link
Member

Tried changing the copy_function to copy instead of copy2 and this did not help. But both preserve permissions, it seems.

@StevenClontz
Copy link
Member Author

I tagged the wrong issue

@oscarlevin
Copy link
Member

Confirming that #655 does fix this for the pretext new, but will not help with core pretext's use of the build in temp, shutil copy out pattern.

@StevenClontz
Copy link
Member Author

PreTeXtBook/pretext#2101 should close this I think then.

@oscarlevin
Copy link
Member

Perhaps I'm being paranoid now, but it seems like core does a lot of things with temporary directories and then copies the result back to the right place. Perhaps implementing the subdirectory trick here instead: https://github.com/PreTeXtBook/pretext/blob/2930c68504dd1c2710e0afbd1e0f785a785c632a/pretext/pretext.py#L3945

@StevenClontz
Copy link
Member Author

good call Oscar - I'll update my PR

@StevenClontz
Copy link
Member Author

Actually that's not quite a one-liner: Rob manually manages a list of temporary directories that he has a cleanup method for, which would not quite work if I returned a subdirectory of the temp directory. So there's a few ways to make it happen, but I'd rather @rbeezer make the call on his end.

@StevenClontz
Copy link
Member Author

Actually nah, I think just coercing permissions for the directory is the right way to go: PreTeXtBook/pretext@d51c9c0

@oscarlevin
Copy link
Member

That works. Or you could return the subdirectory but append the temp directory to the list of directories that will need to be destroyed.

In the mean time though, should we fix the output directory in the CLI? I can work on this.

@StevenClontz
Copy link
Member Author

Yeah - we'll let Rob decide on how he wants this refactored, but I think this works. And in the meantime you've got a patch on our end in the pipeline.

@oscarlevin oscarlevin linked a pull request Dec 22, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants