fix #148 #149

isaid · 2023-09-04T17:03:59+02:00

isaid commented

2023-09-04 17:03:59 +02:00

In check_evobackup_exclude_mount function, add support for new rsync
syntax introduced in evobackup 22.12.

In check_evobackup_exclude_mount function, add support for new rsync syntax introduced in evobackup 22.12.

bwaegeneire approved these changes 2023-09-04 17:07:26 +02:00

bwaegeneire left a comment

Looks good to me!

linux/evocheck.sh Outdated

					
				@ -544,1 +544,3 @@

				                grep -- "--exclude " "${evobackup_file}" | grep -E -o "\"[^\"]+\"" | tr -d '"' > "${excludes_file}"

				                # XXX old release of evobackups don't have version and verions over 22.12 use new syntax to exclude rsync file

				                if grep -q  "^VERSION=" "${evobackup_file}" && test $( sed -E -n 's/VERSION="(.*)"/\1/p' "${evobackup_file}" | tr -d .  ) -ge 2212 ; then 

				                  sed -n '/RSYNC_EXCLUDES="/,/"/p' "${evobackup_file}" | sed '1d;$d' > "${excludes_file}"

bwaegeneire commented

2023-09-04 17:07:00 +02:00

This won't work in some edge cases, for example with:

a first line with RSYNC_EXCLUDES="/foo
a last line with /foo"

I suggest you use sed -En '/RSYNC_EXCLUDES="/,/"/ {s/(RSYNC_EXCLUDES=|")//g;p}' instead.

This won't work in some edge cases, for example with: - a first line with `RSYNC_EXCLUDES="/foo` - a last line with `/foo"` I suggest you use `sed -En '/RSYNC_EXCLUDES="/,/"/ {s/(RSYNC_EXCLUDES=|")//g;p}'` instead.

isaid commented

2023-09-04 17:15:16 +02:00

I fixed and I pushed.

isaid marked this conversation as resolved

isaid force-pushed 148 from 1a832b9bdd to eb9d4875be

2023-09-04 17:13:44 +02:00

Compare

jlecour commented

2023-09-04 18:09:25 +02:00

Hi,

On line 545, I would split the 2 tests.
I would keep the test on evobackup version in the main "if". Then inside this condition I would store the value of the version in a variable, then I would add an "if" on version number.

To compare version numbers, on Debian we can do this with dpkg --compare-versions

Hi, On line 545, I would split the 2 tests. I would keep the test on evobackup version in the main "if". Then inside this condition I would store the value of the version in a variable, then I would add an "if" on version number. To compare version numbers, on Debian we can do this with [`dpkg --compare-versions`](https://manpages.debian.org/bullseye/dpkg/dpkg.1.en.html#compare)

👎 1

isaid force-pushed 148 from d431c3d95f to b31f69a380

2023-09-05 17:13:39 +02:00

Compare

isaid force-pushed 148 from b31f69a380 to b3db84a8f7

2023-09-05 17:21:26 +02:00

Compare

isaid referenced this pull request from a commit

2023-09-05 17:23:15 +02:00

Merge pull request 'fix #148' (#149) from 148 into master

isaid merged commit ed602d9e83 into master

2023-09-05 17:23:16 +02:00

isaid deleted branch 148

2023-09-05 17:23:16 +02:00

mtrossevin commented

2024-01-02 14:37:00 +01:00

Hi,

On line 545, I would split the 2 tests.
I would keep the test on evobackup version in the main "if". Then inside this condition I would store the value of the version in a variable, then I would add an "if" on version number.

To compare version numbers, on Debian we can do this with dpkg --compare-versions

And this proposal broke the check (as described in #150) it would have been great to actually check on really old evobackup versions before implementing this.

The logic needed to either be duplicated or the check sould have been kept in a single if.

As a reminder for future references, long condition chains can be separated in multiple lines if necessary.

> Hi, > > On line 545, I would split the 2 tests. > I would keep the test on evobackup version in the main "if". Then inside this condition I would store the value of the version in a variable, then I would add an "if" on version number. > > To compare version numbers, on Debian we can do this with [`dpkg --compare-versions`](https://manpages.debian.org/bullseye/dpkg/dpkg.1.en.html#compare) And this proposal broke the check (as described in #150) it would have been great to actually check on really old evobackup versions before implementing this. The logic needed to either be duplicated or the check sould have been kept in a single `if`. As a reminder for future references, long condition chains can be separated in multiple lines if necessary.

Sign in to join this conversation.

No reviewers