morgant / 43f Goto Github PK

I'm getting intermittent failure of it_does_not_move_files_within_days_to_keep_directories test in when I run roundup test/lib43f-roll_repository_directories-test.sh on OpenBSD amd64 6.6-stable. I'm not sure if this is some failure of the roll_repository_directories() function, in the test case, or an issue with OpenBSD and/or its file system.

There's a logic bug in detecting wrapping ranges in days to keep in roll_repository_directories() which affects March if preserving more than 28/29 days worth of data (depending on whether it is a leap year).

This is because keep_through will be less than today's date (because 28th/29th through the nonexistent 31st in day in February will be skipped), failing checks for wrapping ranges. Currently, wrapping range detection expects keep_through to be higher than today's date, which won't be the case when keeping more days than the number of days in the previous month (e.g. March following February).

This results in only a couple days' worth of data being preserved, which is the opposite of what is expected.

Many of the conditional statements are long and not very readable, so we should break them up into multiple lines.

As discovered while writing unit tests, roll_repository_monthly_to_monthly() should return false/error when any empty parameters are provided (esp. year, src_month, and dst_month).

If run from a cron or launchd job as root, if dadd is not in the path, 43f run will not correctly create convenience symlinks in init_repository_symlinks(). This can be seen on OpenBSD and macOS (nee OS X). This should cause a whole lot of errors, but this is the most obvious and the first thing that happens in 43f run.

The 43f stats tests (test/43f_stats-test.sh) uses date -j -v commands, which only work on macOS. It looks like we're only doing this to create & init annual directories, so we should be able to do this without even using dadd.

Also, these use stat -f "%k" to determine default block size, but that returns 32768 (at least on my OpenBSD amd64 6.6-stable installation), which is significantly higher than 4096, which is returned on macOS 10.12 Sierra. We're using dd to create files of a specific size, so we should make sure this is safe and correct.

I'm getting consistent failures of the it_does_move_files_outside_months_to_keep_directories test when I run roundup test/lib43f-roll_repository_directories-test.sh on macOS 10.14.x. I did fix a typo related to base conversion, but it persists.

I'm getting a failure of it_does_not_move_files_inside_months_to_keep_directories test when I run roundup test/lib43f-roll_repository_directories-test.sh on OpenBSD amd64 6.6-stable. I'm not 100% sure if this is a bug in the roll_repository_directories() function or a mistake in the test case, but it looks like roll_repository_directories() may be moving files into the wrong month directory.

This may be related to GitHub Issue #12.

When running sudo make install on macOS 10.12 Sierra, I get the following sed error:

sed: 1: "/usr/local/bin/43f": extra characters at the end of l command

The full output is as follows:

cp com.makkintosshu.43f.plist.default /Library/LaunchDaemons/com.makkintosshu.43f.plist

Please run 'sudo launchctl load /Library/LaunchDaemons/com.makkintosshu.43f.plist' to install the LaunchDaemon
mkdir -p /usr/local/bin
install -m755 43f /usr/local/bin
mkdir -p /usr/local/share/43f
install -m755 lib43f /usr/local/share/43f
sed -i 's@source \.@source /usr/local/share/43f@g' /usr/local/bin/43f
sed: 1: "/usr/local/bin/43f": extra characters at the end of l command
make: *** [install] Error 1
hikae:43f-0.2.1 hikaeadmin$ sed -i 's@source \.@source /usr/local/share/43f@g' /usr/local/bin/43f
sed: 1: "/usr/local/bin/43f": extra characters at the end of l command

Upon checking /usr/local/bin/43f, the source line for lib43f was not updated as intended.

As discovered while writing unit tests, roll_repository_daily_to_monthly() should return false/error when any empty parameters are provided (esp. year, day, and month).

The following error is generated because we're not converting to base 10 in valid_month() in lib43f:

/usr/local/share/43f/lib43f: line 226: ((: ( 08: value too great for base (error token is "08")

This likely needs to be fixed in valid_day() too.

When 43f creates new subdirectories in repository (e.g. during 43f init or 43f run, the latter when rolling over to a new year), it uses the default umask instead of maintaining the repository directory's permissions. I often use a custom group with write permissions so that multiple users can write backups into a 43f repository, but must remember to reapply them in the new year or when deploying to a new server. Ideally, 43f would just preserve these permissions automatically.

As discovered while writing unit tests, init_repository_symlinks() should return false if given an uninitialized repository path.

When running roundup test/lib43f-*-test.sh on macOS 10.14.x, I see a lot of failing tests related to format string parsing & replacement:

  it_finds_files_with_test_file_regex:             [FAIL]
  it_finds_files_with_year_regex:                  [FAIL]
  it_replaces_percent_Y_with_year_regex:           [FAIL]
  it_replaces_percent_m_with_month_regex:          [FAIL]
  it_replaces_percent_d_with_day_regex:            [FAIL]
  it_replaces_percent_H_with_hours_regex:          [FAIL]
  it_replaces_percent_M_with_minutes_regex:        [FAIL]
  it_replaces_percent_S_with_seconds_regex:        [FAIL]
  it_replaces_percent_percent_with_percent:        [FAIL]
  it_returns_one_if_filenames_match_and_first_date_greater_than_second_date: [FAIL]
  it_returns_two_if_filenames_match_and_first_date_less_than_second_date: [FAIL]
  it_does_delete_matching_filename_with_older_datestamp: [FAIL]
  it_does_delete_multiple_matching_filenames_with_older_datestamps: [FAIL]
  it_errors_with_blank_output_format_string:       [FAIL]
  it_returns_an_empty_string_with_an_empty_date_format_string: [FAIL]
  it_does_not_modify_string_with_no_date_format_conversion_specifications: [FAIL]
  it_replaces_percent_Y_with_year_regex_in_date_format_string: [FAIL]
  it_replaces_percent_m_with_month_regex_in_date_format_string: [FAIL]
  it_replaces_percent_d_with_day_regex_in_date_format_string: [FAIL]
  it_replaces_percent_H_with_hour_regex_in_date_format_string: [FAIL]
  it_replaces_percent_M_with_minute_regex_in_date_format_string: [FAIL]
  it_replaces_percent_S_with_second_regex_in_date_format_string: [FAIL]
  it_replaces_double_percent_with_single_percent_in_date_format_string: [FAIL]
  it_replaces_multiple_conversion_specifications_in_date_format_string: [FAIL]
  it_returns_full_date_with_empty_format_string:   [FAIL]
  it_returns_empty_string_with_empty_date_string:  [FAIL]
  it_returns_empty_string_with_empty_parse_sequence_string: [FAIL]
  it_parses_four_digit_year_from_date_string:      [FAIL]
  it_parses_two_digit_month_from_date_string:      [FAIL]
  it_parses_two_digit_day_from_date_string:        [FAIL]
  it_parses_two_digit_hour_from_date_string:       [FAIL]
  it_parses_two_digit_minute_from_date_string:     [FAIL]

On OpenBSD, which uses dateutils, we get the following error multiple times if days, months, or years to keep are set to 1 in the config file:

dadd: invalid option -- 0

This is because reldate() is called with days/months/years to keep less one, which would be zero, so reldate is calling dadd with an adjustment of -0d or -0m or -0y, which results in the error.

As discovered while writing unit tests, load_config() will fail with the following error if the notify line contains an email address with a long TLD:

ERROR! 'notify' value in config file is not a valid email address!

Discovered while writing unit tests: if 43f is run in dry run mode (-n/--dry-run), init_repository_symlinks() will still create symlinks.

I'm getting intermittent failure of it_does_not_move_files_modified_today_from_todays_directory test in when I run roundup test/lib43f-roll_repository_directories-test.sh on OpenBSD amd64 6.6-stable. I'm not sure if this is some failure of the roll_repository_directories() function, in the test case, or an issue with OpenBSD and/or its file system.

As discovered while writing unit tests, reldate() should return an error if the format string provided to it doesn't start with a plus (+) symbol.

I'm getting a failure of it_does_move_files_outside_days_to_keep_directories test when I run roundup test/lib43f-roll_repository_directories-test.sh on OpenBSD amd64 6.6-stable. I'm not 100% sure if this is a bug in the roll_repository_directories() function or a mistake in the test case, but it looks like roll_repository_directories() may be moving files into the wrong month directory.

Installation overwrites existing 43f.conf file when run again, for example: when installing an updated version. It should probably back up the configuration file before installing the new config file or not overwrite it if it already exists. Upgrading should be easy and non-destructive.

I'm getting a failure of it_does_move_files_outside_months_to_keep_directories test when I run roundup test/lib43f-roll_repository_directories-test.sh on OpenBSD amd64 6.6-stable. I'm not 100% sure if this is a bug in the roll_repository_directories() function or a mistake in the test case, but it looks like roll_repository_directories() may be moving files into the wrong month directory.

This may be related to GitHub Issues #12 and #13.

We currently implement verbose output as follows:

if $verbose; then echo "some output"; fi

We should be able to do this as follows, which is a little more concise:

$verbose && echo "some output"

As discovered while writing tests, convdatestr() should return an error if given an empty input format string.

Note: I think it currently relies on date to error in other cases, so maybe should explicitly error if any of the inputs are blank.

In version 0.2.x, running 43f stats results in numerous reldate: command not found errors:

/usr/local/bin/43f: line 223: reldate: command not found                                         
/usr/local/bin/43f: line 223: reldate: command not found                                         
/usr/local/bin/43f: line 223: reldate: command not found                                         
/usr/local/bin/43f: line 223: reldate: command not found
/usr/local/bin/43f: line 279: reldate: command not found
/usr/local/bin/43f: line 279: reldate: command not found
/usr/local/bin/43f: line 279: reldate: command not found
/usr/local/bin/43f: line 279: reldate: command not found
/usr/local/bin/43f: line 335: reldate: command not found
/usr/local/bin/43f: line 335: reldate: command not found
/usr/local/bin/43f: line 335: reldate: command not found
/usr/local/bin/43f: line 335: reldate: command not found