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.

Sign up for GitHub

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

Jump to bottom

Add creation_time API #134

Closed
wants to merge 23 commits into from
Closed

Add creation_time API #134

wants to merge 23 commits into from

Conversation

soroshsabz
Copy link

@soroshsabz soroshsabz commented Apr 11, 2020 •
edited
Loading

ITNOA

This PR resolved #133

I added support for below platform

  • windows
  • linux with glibc greater than 2.28
  • Linux with glibc less than 2.28
  • FreeBSD
  • Mac OSX greater than Mac OS X 10.5 (Leopard)

I add another platform in coming days.

I would be happy to review my PR.

thanks a lot

Lastique
Lastique reviewed Apr 12, 2020
View reviewed changes
test/operations_test.cpp Outdated Show resolved Hide resolved
@soroshsabz
Copy link
Author

soroshsabz commented Apr 14, 2020 •
edited
Loading

Finally I can make green Linux tests in travis :) the only one build remaining in travis, it is Mac OSX

@soroshsabz soroshsabz changed the title Add creation_time API [WIP] Add creation_time API Apr 16, 2020
@soroshsabz
Copy link
Author

@Lastique Thanks for review my code I try to resolve all comments and support all desired platforms. so I happy to review it again.

Thanks

@soroshsabz soroshsabz mentioned this pull request Apr 18, 2020
Closed
@soroshsabz
Copy link
Author

@Lastique I need this feature, so I thankful for review it :)

Lastique
Lastique requested changes May 1, 2020
View reviewed changes
src/operations.cpp Outdated Show resolved Hide resolved
src/linux_statx.hpp Outdated Show resolved Hide resolved
src/operations.cpp Outdated Show resolved Hide resolved
src/operations.cpp Outdated Show resolved Hide resolved
test/operations_test.cpp Outdated Show resolved Hide resolved
src/operations.cpp Outdated Show resolved Hide resolved
src/operations.cpp Show resolved Hide resolved
src/operations.cpp
if (error(::stat(p.c_str(), &path_stat) != 0 ? BOOST_ERRNO : 0, p, ec,
"boost::filesystem::creation_time"))
return std::time_t(-1);
return std::time_t(path_stat.st_birthtim.tv_sec);
Copy link
Member

Choose a reason for hiding this comment

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

I think, st_birthtime is a more portable way to access this field instead of st_birthtim.tv_sec. Also, NetBSD seems to support it as well: https://netbsd.gw.com/cgi-bin/man-cgi?stat+2.i386+NetBSD-8.0

Copy link
Author

Choose a reason for hiding this comment

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

it is only work, when _POSIX_SOURCE not defined, so I think it is dangerous to use this.

Please see https://www.freebsd.org/cgi/man.cgi?query=stat&sektion=2

test/operations_test.cpp Outdated Show resolved Hide resolved
test/operations_test.cpp Outdated Show resolved Hide resolved
@soroshsabz
Copy link
Author

@Lastique I am sorry for my long delay, but I think I resolves many of your comment, and you can review it again.

thanks a lot

Lastique added a commit that referenced this pull request Aug 19, 2020
@Lastique
Added creation_time operation.
1eec6f4 
The operation allows to query file creation time.

Implementation partially inspired by:

#134

Closes #134.
@Lastique
Copy link
Member

Thanks for your work. I have looked through your code and there were issues with it. For example, you are not checking the stx_mask field to see if the requested data was actually retrieved. Also, AT_SYMLINK_NOFOLLOW is not correct, as the function has to resolve symlinks, like last_write_time does. There were other issues as well, so I decided to implement this myself, taking some bits of your PR as an example. My current version is here.

@Lastique Lastique closed this Aug 19, 2020
Lastique added a commit that referenced this pull request Aug 19, 2020
@Lastique
Added creation_time operation.
a7ec1ab 
The operation allows to query file creation time.

Implementation partially inspired by:

#134

Closes #134.
Lastique added a commit that referenced this pull request Aug 19, 2020
@Lastique
Added creation_time operation.
2a1c7a7 
The operation allows to query file creation time.

Implementation partially inspired by:

#134

Closes #134.
Lastique added a commit that referenced this pull request Aug 20, 2020
@Lastique
Added creation_time operation.
a031e4f 
The operation allows to query file creation time.

Implementation partially inspired by:

#134

Closes #134.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lastique Lastique Awaiting requested review from Lastique

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add creation_time API in operations.hpp
2 participants
@soroshsabz @Lastique