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

Preserve timestamps and permissions #179

Merged
merged 2 commits into from
Jun 23, 2015

Conversation

vincentbernat
Copy link
Contributor

Add a --preserve option to keep both timestamps and permissions. This closes #31, except for the owner part. I don't think that preserving the owner is really important as it is only possible while being root and this should not be really a use case for unoconv.

I have only tested with Python3 has I don't have UNO binding for Python 2 anymore but the code should be compatible with Python 2. Please test if you can.

The `--preserve-timestamp` flag can now be used to copy the timestamp of
the original file to the converted file. This partly fix unoconv#31.
@@ -1,4 +1,4 @@
#!/usr/bin/env python
#!/usr/bin/env python3
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there really a need for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the modification is compatible with both Python 2.x and Python 3.x. I just tested with Python 3.x last.

Copy link
Contributor

Choose a reason for hiding this comment

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

@vincentbernat Very nice! Can you put back original shebang? Because it's not tested well on python 3 to do such risky changes and there is not needs for that PR.
And I will be ready to merge it!

@raphink
Copy link
Contributor

raphink commented Mar 10, 2014

👍

And change its name to `--preserve`. This is mostly what is requested
in unoconv#31.
@vincentbernat
Copy link
Contributor Author

OK, just updated!

@skywinder
Copy link
Contributor

Great! I will handle it!

@skywinder
Copy link
Contributor

@dagwieers please, take a look for this update also. It's simple, but sometimes can be useful.

@dagwieers
Copy link
Member

Oh my, I didn't see this PR at all. This is definitely something we needed, and I like the implementation as well !

And to be honest, I think this should be the default (expected) behavior. Or are there any use-cases where we don't want to do this ? (Security-wise I'd expect the permissions to be exactly as they were and not use umask at all)

The only reason for not doing this by default to me is the fact that other tools also make this optional. And that is a good reason too.

PS A short option would be convenient as well if this is not the default behavior, unfortunately -p is already taken (I wish it wasn't though, --port would have sufficed)

dagwieers added a commit that referenced this pull request Jun 23, 2015
@dagwieers dagwieers merged commit e86efd5 into unoconv:master Jun 23, 2015
@dagwieers
Copy link
Member

Thanks a lot !

@dagwieers dagwieers added this to the Release 0.7 milestone Jul 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Preserve the original timestamp, ownership and permissions
4 participants