#1052 PR `merged`: New Borg variables introduced¶

Labels: enhancement, fixed / solved / done

gozora opened issue at 2016-10-27 06:31:¶

New ReaR variables for controlling Borg behaviour:

BORGBACKUP_KEYS_DIR
BORGBACKUP_CACHE_DIR
BORGBACKUP_RELOCATED_REPO_ACCESS_IS_OK
BORGBACKUP_UNKNOWN_UNENCRYPTED_REPO_ACCESS_IS_OK

jsmeix commented at 2016-10-27 08:52:¶

I really appreciate variable names that actually explain
what they do, even if it makes them very long.
In case of doubt better (even annoyingly) long and explanatory
than a bit too short and user issues when they misunderstand it.

@gozora
many thanks for your continuous Borg Backup integration work!

gozora commented at 2016-10-27 08:57:¶

@jsmeix
Pleasure ;-)

Regarding comment, did you though about linking ReaR with borgbackup/community repo?

jsmeix commented at 2016-10-27 08:58:¶

@gozora
I wonder if in ReaR really separated variable names
must be used when all what actually happens is

export BORG_name=$BORGBACKUP_name

Woudn't it be simpler and more straightforward to specify
in ReaR the value for BORG_name directly in such cases?

jsmeix commented at 2016-10-27 09:02:¶

I noticed
https://github.com/borgbackup/borg/issues/1702#issuecomment-255441289
but I found no time to look at the borgbackup/community repo
how to do it exactly (in my other life I am currently fighting
with various issues in and related to Ghostscript ;-)

gozora commented at 2016-10-27 09:16:¶

@jsmeix

I wonder if in ReaR really separated variable names
must be used when all what actually happens is

export BORG_name=$BORGBACKUP_name
Woudn't it be simpler and more straightforward to specify
in ReaR the value for BORG_name directly in such cases?

What I was aiming for was possibility for user to use BORG_* variables from shell (for whatever reason) like BORG_REMOTE_PATH=/tmp/borg rear mkbackup and not collide with ReaR configuration.
Such configuration looks more straightforward to me.

gozora commented at 2016-10-27 10:24:¶

@jsmeix
Ah printing interesting topic ;-)

I've created this.

I can create pull request if you want ...

jsmeix commented at 2016-10-27 10:36:¶

I meant when rear (the running program) sets BORG_* variables
directly then this should not collide with user's settings
because BORG_* variables settings "inside" rear are
independent from "outside" user's settings.

What not works with your approach is

BORG_RELOCATED_REPO_ACCESS_IS_OK="n" rear mkbackup

because you overwrite that with what is specified in default.conf

BORGBACKUP_RELOCATED_REPO_ACCESS_IS_OK="y"

and the subsequent

if [ ! -z $BORGBACKUP_RELOCATED_REPO_ACCESS_IS_OK ]; then
    export BORG_RELOCATED_REPO_ACCESS_IS_OK=$BORGBACKUP_RELOCATED_REPO_ACCESS_IS_OK
    fi

When what is specified in default.conf has precedence over
existing user environment settings, then I think you can set
BORG_RELOCATED_REPO_ACCESS_IS_OK directly
in default.conf.

From my current point of view using BORGBACKUP_name
for BORG_name variables could be only a useless level
of indirection (cf. RFC 1925 item 6a).

But I am not a Borg Backup user, so that I could be plain wrong.

jsmeix commented at 2016-10-27 10:38:¶

Typo correction:
I meant what not works with your approach is

BORG_RELOCATED_REPO_ACCESS_IS_OK="n" rear mkbackup

when there is in default.conf

BORGBACKUP_RELOCATED_REPO_ACCESS_IS_OK="y"

gozora commented at 2016-10-27 11:08:¶

@jsmeix
Variables you've took as an example are kind of special.
They purpose is to give automatic reply to Borg during confirmation dialog, hence they prevent your backup/restore to be temporarily suspended. That was the reason why I've set defaults to accept ("y").
(User can override then from shell if BORGBACKUP_RELOCATED_REPO_ACCESS_IS_OK="" is set in local.conf, yes agree that is stupid, but possible RFC 1925 point 3 :-)).

One more reason we have this indirection is discussion.

In my opinion, this gives us more maneuvering space for mapping Borg variables to ReaR and also have bigger control over certain aspects of Borg behavior.

jsmeix commented at 2016-10-27 13:12:¶

I would be grateful if you do the pull request
for borgbackup/community

Regarding BORGBACKUP_name versus BORG_name:
Feel free to implement it as it works best for you.
Only a note FYI:
I thought ReaR uses BORGBACKUP_* to keep ReaR's own
stuff related to Borg Backup separated from Borg's own stuff.
But in the above mentioned case BORGBACKUP_name is only
another name for BORG_name and then I was thinking about
why not using BORG_name directly.

gozora commented at 2016-10-27 13:52:¶

There are actually two types of Borg variables used in implementation:

Variables passed to borg as parameter e.g.:
BORGBACKUP_REMOTE_PATH, BORGBACKUP_ENC_TYPE ...
These are used to build argument list for borg, and can't be passed as environment variables.
We had discussion about this group and changed prefix from BORG_name -> BORGBACKUP_name
Environment variables: e.g.:
BORGBACKUP_CACHE_DIR, BORGBACKUP_KEYS_DIR ..., which are read by borg as env variables.
This group was introduced later and I thought that it could be better to align naming conventions with first group, to avoid having both (BORG_name and BORGBACKUP_name) in default.conf.

jsmeix commented at 2016-10-27 14:09:¶

This difference is what I mean.

Those under (1.) are from my point of view ReaR's
own stuff that is only related to Borg Backup while
those under (2.) are from my point of view Borg's own stuff.

My personal thinking is that Borg's own environment variables
should be used directly like all other environment variables.

From my point of view it is correct to set and use in ReaR
(e.g in default.conf, local.conf or in whatever ReaR script)
environment variables directly with their original name, e.g.
TMPDIR in default.conf (cf. https://github.com/rear/rear/issues/968)
or things like LC_ALL and LANG in usr/sbin/rear and so on.

gozora commented at 2016-10-27 15:27:¶

Ah, maybe I got your point now (what a looong wiring I have :-)).
You think it would be better to drop everything starting with line 49 until line 71 in lib/borg-functions.sh and let user define Borg native variables in local.conf like:

BORGBACKUP_REMOTE_PATH="/usr/local/bin/borg"
BORGBACKUP_ENC_TYPE="keyfile"

# Borg env vars
export BORG_PASSPHRASE="S3cr37_P455w0rD"
export BORG_RELOCATED_REPO_ACCESS_IS_OK="y"
...

Is that correct?

jsmeix commented at 2016-10-27 16:13:¶

@gozora
first and foremost an important note:

Since I merged https://github.com/rear/rear/pull/1053
basically ALL scripts got a changed name
from NM_name.sh to NM0_name.sh
(except 00_name.sh that is now 005_name.sh)

This means you must update your working copy
to the current master state otherwise I assume
you will get big conflicts with further pull requests.

gozora commented at 2016-10-27 16:15:¶

@jsmeix are you working overtimes today? :-)
Thanks for notification!

jsmeix commented at 2016-10-27 16:20:¶

@gozora regarding your
https://github.com/rear/rear/pull/1052#issuecomment-256675010

Yes that is what I have in mind.

And no - you do not have a long wiring - it is complicated
to get all those various stuff done sufficiently well.
I do appreciate it that you listen to my comments and
always respond usefully - I am also learning a lot - and
it does not at all matter whether you are right or I am right,
what matters is that in the end the result is sufficiently o.k.

jsmeix commented at 2016-10-27 16:22:¶

@gozora
I was working "undertimes" the other days
but today I can work longer which is good because
I did not expect that https://github.com/rear/rear/pull/1053
was almost a full-day job (and now my brain feels empty ;-)

gozora commented at 2016-10-27 16:51:¶

Created pull-request in borgbackup/community.
Hope it is OK for everybody.

gozora commented at 2016-10-27 17:57:¶

@jsmeix so I've created #1055 which (except some documentation updates) pretty much cancels this pull request :-D.

[Export of Github issue for rear/rear.]

#1052 PR merged: New Borg variables introduced¶