organicmaps/organicmaps-tmp

Archived

Fork 1

[linux] Approximate create time for older Linux versions #4256

Merged

biodranik merged 2 commits from linux/create-time into master

2023-01-12 06:55:14 +00:00

biodranik commented

2023-01-10 14:48:37 +00:00

(Migrated from github.com)

@pastk please check if it works on your system.

pastk requested changes 2023-01-10 15:31:36 +00:00

pastk left a comment

Owner

It compiles and runs on my system now (with my return st.st_mtim.tv_sec; change)!

Though I don't know whether and how it affects the functionality which uses this creation date...

It compiles and runs on my system now (with my `return st.st_mtim.tv_sec;` change)! Though I don't know whether and how it affects the functionality which uses this creation date...

platform/platform_linux.cpp

pastk commented

2023-01-10 15:17:25 +00:00

Owner

    return st.st_mtim.tv_sec;

ctime and atime can not be older than m_time.

Also I wonder why in platform_android.cpp atime is used and not mtime?

(note that field names are e.g. st_atim, not st_atime)

```suggestion return st.st_mtim.tv_sec; ``` `ctime` and `atime` can not be older than `m_time`. Also I wonder why in `platform_android.cpp` `atime` is used and not `mtime`? (note that field names are e.g. `st_atim`, not `st_atime`)

pastk commented

2023-01-10 15:22:08 +00:00

Owner

I wonder if it'll be better to check directly if statx() is defined instead of relying on glibc version? Is it possible some glibc replacement like musl, etc. is used?

I wonder if it'll be better to check directly if `statx()` is defined instead of relying on glibc version? Is it possible some glibc replacement like musl, etc. is used?

biodranik (Migrated from github.com) reviewed 2023-01-10 16:40:49 +00:00

platform/platform_linux.cpp

biodranik (Migrated from github.com) commented

2023-01-10 16:40:49 +00:00

What was the compilation error?
Why did you decide to use modification time? It is not correct approach. There is no correct approach for Linux, we can only hope that it may work in some cases (see 2 below). An appropriate fix would be to use corresponding tv_sec values.
Android:

The Access time would be the creation date time if the partition is mounted with option noatime, which is the factory default for all 3 of my android phones. That is to say, practically, you can check file/folder creation time.

0. What was the compilation error? 1. Why did you decide to use modification time? It is not correct approach. There is no correct approach for Linux, we can only hope that it may work in some cases (see 2 below). An appropriate fix would be to use corresponding tv_sec values. 2. Android: > The Access time would be the creation date time if the partition is mounted with option noatime, which is the factory default for all 3 of my android phones. That is to say, practically, you can check file/folder creation time.

pastk reviewed 2023-01-10 18:16:24 +00:00

platform/platform_linux.cpp

pastk commented

2023-01-10 18:16:24 +00:00

Owner

What was the compilation error?

/home/pastk/dev/organicmaps/platform/platform_linux.cpp: In static member function ‘static time_t Platform::GetFileCreationTime(const string&)’:
/home/pastk/dev/organicmaps/platform/platform_linux.cpp:279:24: error: ‘struct stat’ has no member named ‘atime’
     return std::min(st.atime, std::min(st.st_mtime, st.st_ctime));
                        ^~~~~

2. Android:
The Access time would be the creation date time if the partition is mounted with option noatime, which is the factory default for all 3 of my android phones. That is to say, practically, you can check file/folder creation time.

Yeap you're right atime could be older than mtime in noatime case.
Still, ctime can not be older, so no point in checking it.

> 0. What was the compilation error? ``` /home/pastk/dev/organicmaps/platform/platform_linux.cpp: In static member function ‘static time_t Platform::GetFileCreationTime(const string&)’: /home/pastk/dev/organicmaps/platform/platform_linux.cpp:279:24: error: ‘struct stat’ has no member named ‘atime’ return std::min(st.atime, std::min(st.st_mtime, st.st_ctime)); ^~~~~ ``` > 2. Android: > > > > The Access time would be the creation date time if the partition is mounted with option noatime, which is the factory default for all 3 of my android phones. That is to say, practically, you can check file/folder creation time. Yeap you're right `atime` could be older than `mtime` in `noatime` case. Still, `ctime` can not be older, so no point in checking it.

biodranik (Migrated from github.com) reviewed 2023-01-10 18:20:25 +00:00

platform/platform_linux.cpp

biodranik (Migrated from github.com) commented

2023-01-10 18:20:24 +00:00

As you've seen, atime may be earlier.

ctime may also be earlier than mtime and atime (if it's enabled), because it changes only when file's metadata changes, not the content.

I'm still convinced that my proposed implementation is more robust.

As you've seen, atime may be earlier. ctime may also be earlier than mtime and atime (if it's enabled), because it changes only when file's metadata changes, not the content. I'm still convinced that my proposed implementation is more robust.

pastk reviewed 2023-01-10 18:35:36 +00:00

platform/platform_linux.cpp

pastk commented

2023-01-10 18:35:36 +00:00

Owner

ctime is updated too when file's contents are changed.

`ctime` is updated too when file's contents are changed.

biodranik (Migrated from github.com) reviewed 2023-01-10 21:14:52 +00:00

platform/platform_linux.cpp

biodranik (Migrated from github.com) commented

2023-01-10 21:14:52 +00:00

syscall can be used, but that's way too complex. That function is not critical for Linux, and the result of calling it on older Linuxes/file systems without birth time is not reliable anyway. let's compile it now and move on.

biodranik (Migrated from github.com) reviewed 2023-01-10 21:16:33 +00:00

platform/platform_linux.cpp

biodranik (Migrated from github.com) commented

2023-01-10 21:16:33 +00:00

Ok, double-checked it. Agree, let's remove ctime check.

biodranik (Migrated from github.com) reviewed 2023-01-10 21:17:07 +00:00

platform/platform_linux.cpp

biodranik (Migrated from github.com) commented

2023-01-10 21:17:06 +00:00

    return std::min(st.t_atim.tv_sec, st.t_mtim.tv_sec);

```suggestion return std::min(st.t_atim.tv_sec, st.t_mtim.tv_sec); ```

pastk approved these changes 2023-01-10 22:17:55 +00:00

platform/platform_linux.cpp

pastk commented

2023-01-10 22:17:41 +00:00

Owner

    return std::min(st.st_atim.tv_sec, st.st_mtim.tv_sec);

```suggestion return std::min(st.st_atim.tv_sec, st.st_mtim.tv_sec); ```

This repo is archived. You cannot comment on pull requests.

No reviewers

pastk

No labels

Android Automotive (AAOS)

API

AppGallery

AppStore

Battery and Performance

External Map Datasets

F-Droid

Fonts

Frequently User Reported

No milestone

No project

No assignees

2 participants

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: organicmaps/organicmaps-tmp#4256

No description provided.

Rows
Columns