Make LogCleaner support relative paths

[aesophor]

Closes #653

In #653, the user reports that the following usage of glog API:

google::SetLogDestination(google::INFO, ".");
google::EnableLogCleaner(1);

will lead to an uncaught out_of_range exception.

terminate called after throwing an instance of 'std::out_of_range'
  what():  basic_string::at: __n (which is 18446744073709551615) >= this->size() (which is 0)
[1]    835009 abort (core dumped)  ./out

This is caused by a bug in LogCleaner::Run() where base_filename doesn't end with a "/".

glog/src/logging.cc

Lines 1334 to 1335 in 0b3d4cb

	string dir = base_filename.substr(0, base_filename.find_last_of(dir_delim_) + 1);
	dirs.push_back(dir);

This PR:

1. checks if base_filename in LogCleaner::Run() ends with a "/", and if not, then we simply pass the relative path to LogCleaner::GetOverdueLogNames().
2. In LogCleaner::GetOverdueLogNames, we only concatenate log_directory and ent->d_name if log_directory ends with a "/". Otherwise, just pass the relative path to LogCleaner::IsLogFromCurrentProject.
3. Some trivial changes to improve the readability of source code

[aesophor]

@sergiud @jixianliu1234

I've also discovered this:

google::SetLogDestination(google::INFO, "./");  // creates logs under process's cwd
                                                // example: 20210518-122553.954758

google::SetLogDestination(google::INFO, ".");   // also creates logs under process's cwd, but filenames starts with "."
                                                // example: .20210518-122537.954584

Another example:

google::SetLogDestination(google::INFO, "/tmp/");  // creates logs under /tmp/
google::SetLogDestination(google::INFO, "/tmp");   // creates logs under / with filenames starting with "tmp"

Therefore, I think the correct way of setting log destination to "." is google::SetLogDestination(google::INFO, "./");.
Maybe this should be documented somewhere, so that users won't accidentally write google::SetLogDestination(google::INFO, "."); and expect the logs to all go to the current working directory.

Please tell me if I've missed anything in the documentation. Thanks.

[sergiud]

I believe there should be no difference between

google::SetLogDestination(google::INFO, "./");

and

google::SetLogDestination(google::INFO, ".");

since . and ./ are both directories. However, this will require decomposing the path to determine the directory and the base filename.

[sergiud]

at throws an out of range access. This should be avoided.

[sergiud]

These formatting changes seem to be unrelated. I would undo them.

[sergiud]

Any progress?

[sergiud]

@aesophor There are some conflicts now. Could you please rebase?

[coveralls]

Coverage increased (+0.2%) to 76.611% when pulling c27c3a8 on aesophor:fix-path into e8e40f7 on google:master.

[aesophor]

@aesophor There are some conflicts now. Could you please rebase?

@sergiud No problem! I'll get back to this ASAP (probably at the beginning of November). Thanks!

[google-cla]

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

[sergiud]

@googlebot I consent.

[sergiud]

@aesophor Thanks. That's not necessary anymore; I already addressed all the open points.

After careful consideration, the basenames . and ./ must not be treated equally to refer to the current directory. To set a directory, the log_dir flag should be used instead. If glog were to treat the aforementioned basenames the same, it would not be possible to use . as a prefix (e.g., for hidden log files).

Although not directly related to this PR, I encountered some problems writing unit tests for the log cleaner:

• Why can't the number of (overdue) days be 0? Removing all but the latest log seems to be a perfectly reasonable use case to me.
• Successively starting the same process which immediately exits does not trigger the log cleaner (overdue days = 0). However, if I delay the process start e.g., by a second the log cleaner kicks in as expected. Why is that?