Skip to content

Copy file Attributes#2286

Closed
Siedlerchr wants to merge 1 commit into
masterfrom
copyPermissions
Closed

Copy file Attributes#2286
Siedlerchr wants to merge 1 commit into
masterfrom
copyPermissions

Conversation

@Siedlerchr

@Siedlerchr Siedlerchr commented Nov 18, 2016 •
edited
Loading

Copy link
Copy Markdown
Member

Should fix #2285 and #2279 , too.
Edit// Needs Testing for #2285

Remove PosixFilePermission, as this will throw an Exception on Win

  • Change in CHANGELOG.md described
  • Tests created for changes
  • Screenshots added (for bigger UI changes)
  • Manually tested changed features in running JabRef
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)
  • If you changed the localization: Did you run gradle localizationUpdate?

Add Copy_Attributes flag

@Siedlerchr Siedlerchr mentioned this pull request Nov 18, 2016
Merged
6 tasks
tobiasdiez
tobiasdiez reviewed Nov 18, 2016
View reviewed changes
Comment thread src/main/java/net/sf/jabref/logic/util/io/FileUtil.java
try {
return Files.copy(pathToSourceFile, pathToDestinationFile, StandardCopyOption.REPLACE_EXISTING) != null;
return Files.copy(pathToSourceFile, pathToDestinationFile, StandardCopyOption.REPLACE_EXISTING,
StandardCopyOption.COPY_ATTRIBUTES) != null;

@tobiasdiez tobiasdiez Nov 18, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This code ensures that the file permission of the temporary file are kept in https://github.com/JabRef/jabref/pull/2286/files#diff-4925b1e29b2566af2796f9c99008b9ddR86, but as far as I can see there is no guarantee that the temporary file has the same permission attributes as the original file.

@Siedlerchr Siedlerchr Nov 18, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@tobiasdiez I am not sure either. However, the problem is that PosixPermission will fail on Windows with an exception

@Siedlerchr

Siedlerchr commented Nov 18, 2016

Copy link
Copy Markdown
Member Author

Okay, permissons are not copied. Maybe it is enough to check for Unix OS/POSIX support:
https://docs.oracle.com/javase/tutorial/essential/io/fileAttr.html

@tobiasdiez

tobiasdiez commented Nov 18, 2016

Copy link
Copy Markdown
Member

Yes I think checking

private static final boolean isPosixCompilant = FileSystems.getDefault().supportedFileAttributeViews().contains("posix");

should be enough. Do you want to add this, or should I?

@Siedlerchr

Siedlerchr commented Nov 18, 2016

Copy link
Copy Markdown
Member Author

Please do add this. I am answering from phone

2016-11-18 23:51 GMT+01:00 Tobias Diez notifications@github.com:

Yes I think checking

private static final boolean isPosixCompilant = FileSystems.getDefault().
supportedFileAttributeViews().contains("posix");

should be enough. Do you want to add this, or should I?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#2286 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AATi5IAtK7Nnx-qm999U3pYZV7FQSgNlks5q_ivvgaJpZM4K23M8
.

@Siedlerchr Siedlerchr closed this Nov 24, 2016
@Siedlerchr Siedlerchr deleted the copyPermissions branch November 24, 2016 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tobiasdiez tobiasdiez tobiasdiez left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Cannot complete save of database with empty key(s)

2 participants

@Siedlerchr @tobiasdiez