Example to hit: func (oev optionEnvVars) Get(key string)

Ole · May 11, 2021, 7:49am

I am debugging/fixing an issue with environment variables (--stats, –separator, --log-level, --multi-thread-streams, --rc-…)

As part of the fix I have added debug logging to show when flags/parameters are being read from environment variables.

I am now in the process of testing my changes, but there is one Get function that I cannot find out how to reach.

Can somebody help me with a simple example to hit an entry breakpoint in the below function:

github.com

rclone/rclone/blob/732bc08ceda0d6a4432d512277b77090b8ae3b79/fs/fs.go#L1281-L1295


// Get a config item from the option environment variables if possible
func (oev optionEnvVars) Get(key string) (value string, ok bool) {
	opt := oev.fsInfo.Options.Get(key)
	if opt == nil {
		return "", false
	}
	// For options with NoPrefix set, check without prefix too
	if opt.NoPrefix {
		value, ok = os.LookupEnv(OptionToEnv(key))
		if ok {
			return value, ok
		}
	}
	return os.LookupEnv(OptionToEnv(oev.fsInfo.Prefix + "-" + key))
}

and even better two examples; one with NoPrefix and one without.

I guess it is being used in the backend configuration, but have not been able to find the right remote/flag combination.

Thanks in advance!

ncw · May 11, 2021, 8:55am

Those are for config by env vars.

So set RCLONE_LOCAL_LINKS or RCLONE_LINKS would be an example with and without prefix to use the --links from the local backend.

Or more straight forwardly RCLONE_DRIVE_USE_TRASH

Ole · May 11, 2021, 4:02pm

Thanks, I hit my breakpoint

In the process I discovered a bug/less intuitive setting precedence.

Here is an example to reproduce the issue:

RCLONE_SKIP_LINKS = true                 #superseeds the rclone default
RCLONE_LOCAL_SKIP_LINKS = false          #intuitively supersedes the above
RCLONE_CONFIG_MYLOCAL_SKIP_LINKS = false #intuitively supersedes both above
rclone lsd myLocal:/folder/with/symlinks -vv

Intuitively expected result: folder listing with symlink warnings
Actual result: folder listing without symlink warnings (RCLONE_SKIP_LINKS has precedence)

The precedence is described here, and it actually doesn’t describe the above situation.

Does the result correspond to your expectation?

Do we aim a clarifying the docs or fixing the code?

(You may now be thinking about using flag.Changed, to differ between flags set by the command line and the environment variables. But this difference is exactly what caused the issues I have just fixed - by making sure that flag.Changed is also being set by environment variables )

ncw · May 11, 2021, 8:44pm

The config precedence is defined here

github.com

rclone/rclone/blob/ce83228cb21e6839c105b55d71c50cab671e4ed6/fs/fs.go#L1349


	}
	return value, ok
}


// ConfigMap creates a configmap.Map from the *RegInfo and the
// configName passed in. If connectionStringConfig has any entries (it may be nil),
// then it will be added to the lookup with the highest priority.
//
// If fsInfo is nil then the returned configmap.Map should only be
// used for reading non backend specific parameters, such as "type".
func ConfigMap(fsInfo *RegInfo, configName string, connectionStringConfig configmap.Simple) (config *configmap.Map) {
	// Create the config
	config = configmap.New()


	// Read the config, more specific to least specific


	// Config from connection string
	if len(connectionStringConfig) > 0 {
		config.AddGetter(connectionStringConfig, configmap.PriorityNormal)
	}

It appears to be in the order of the docs but I think set to the default is being conflated with not set.

I have a feeling that this was recently introduced - can you try some earlier rclone versions and see if that is the case?

Ole · May 12, 2021, 1:32pm

I have reproduced the issue in rclone 1.54 (02-Feb-2021) and performed a high level debugging of the configuration flow in the latest master.

The root cause seems to be that unspecific (command line) flags takes precedence over remote and backend specific flags. Here is an example to illustrate the dilemma:

RCLONE_CONFIG_MYLOCAL_SKIP_LINKS = false
rclone lsd myLocal::/folder/with/symlinks --skip-links
# Actual result: Folder list without warnings

That is, the unspecific --skip-links on the command line (or from the environment) takes precedence over the very specific RCLONE_CONFIG_MYLOCAL_SKIP_LINKS.

This actually also corresponds to the sequence in func ConfigMap where flag values get preference over remote and backend specific flags:

github.com

rclone/rclone/blob/ce83228cb21e6839c105b55d71c50cab671e4ed6/fs/fs.go#L1353-L1371


	// Read the config, more specific to least specific


	// Config from connection string
	if len(connectionStringConfig) > 0 {
		config.AddGetter(connectionStringConfig, configmap.PriorityNormal)
	}


	// flag values
	if fsInfo != nil {
		config.AddGetter(&regInfoValues{fsInfo, false}, configmap.PriorityNormal)
	}


	// remote specific environment vars
	config.AddGetter(configEnvVars(configName), configmap.PriorityNormal)


	// backend specific environment vars
	if fsInfo != nil {
		config.AddGetter(optionEnvVars{fsInfo: fsInfo}, configmap.PriorityNormal)
	}

My best guess is that the issue has been there for 2-3 years.

A bold guess is that the issue can be fixed by moving the code to add the flag values just after the code to add the remote and backend specific flags.

That change will most likely reveal a bug in the following code, where the (more specific) prefixed flag should be checked before the non-prefixed:

github.com

rclone/rclone/blob/ce83228cb21e6839c105b55d71c50cab671e4ed6/fs/fs.go#L1281-L1295


// Get a config item from the option environment variables if possible
func (oev optionEnvVars) Get(key string) (value string, ok bool) {
	opt := oev.fsInfo.Options.Get(key)
	if opt == nil {
		return "", false
	}
	// For options with NoPrefix set, check without prefix too
	if opt.NoPrefix {
		value, ok = os.LookupEnv(OptionToEnv(key))
		if ok {
			return value, ok
		}
	}
	return os.LookupEnv(OptionToEnv(oev.fsInfo.Prefix + "-" + key))
}

I don’t mind giving it a try, if it sounds reasonable to your

(Please note: The full overview of consequences and need for testing is above my head)

ncw · May 12, 2021, 8:54pm

Ole:

The root cause seems to be that unspecific (command line) flags takes precedence over remote and backend specific flags. Here is an example to illustrate the dilemma:
RCLONE_CONFIG_MYLOCAL_SKIP_LINKS = false
rclone lsd myLocal::/folder/with/symlinks --skip-links
# Actual result: Folder list without warnings

I think this is working as documented - the config flags take precedence over ENV vars.

This is quite a different example to the one above.

I don't want to change the precedence of the command line flags - that is too risky to break people's stuff!

However I note that you can use the connection string syntax to specify config which will work specifically and exactly how you want it (one of the reasons I made it).

rclone lsd myLocal,skip_links:/folder/with/symlinks

Ole · May 17, 2021, 12:16pm

That's certainly a valid point!

Sorry, but that doesn’t seem to be the situation. Let me illustrate with a fatal example:

    RCLONE_DRIVE_USE_TRASH='false'
    RCLONE_CONFIG_MYDRIVE_USE_TRASH='true'
    rclone touch myDrive:test-trash
    rclone lsl myDrive: --include "test-trash" 
    rclone deletefile myDrive:test-trash
    rclone lsl myDrive,trashed_only: --include "test-trash"

Result according to the docs: A recoverable file in the trash folder.

Actual result: The file isn’t in the trash folder and cannot be recovered.

The actual behavior corresponds to this:

The various different methods of backend configuration are read in this order and the first one with a value is used.

Parameters in connection strings, e.g. myDrive,use_trash:

Flag values as supplied on the command line, e.g. --drive-use-trash.

Command line flags set by environment vars, e.g. RCLONE_DRIVE_USE_TRASH.

Remote specific environment vars, e.g. RCLONE_CONFIG_MYREMOTE_USE_TRASH (see above).

Backend specific environment vars, e.g. RCLONE_DRIVE_USE_TRASH.

Config file, e.g. use_trash = false.

Default values, e.g. true - these can't be changed.

The additions to the current documentation are marked in italic.
The condition taking precedence in the above example is marked in bold.

ncw · May 17, 2021, 2:43pm

Ole:

Sorry, but that doesn’t seem to be the situation. Let me illustrate with a fatal example:

    RCLONE_DRIVE_USE_TRASH='false'
    RCLONE_CONFIG_MYDRIVE_USE_TRASH='true'
    rclone touch myDrive:test-trash
    rclone lsl myDrive: --include "test-trash" 
    rclone deletefile myDrive:test-trash
    rclone lsl myDrive,trashed_only: --include "test-trash"

I did a little test to verify what you are saying - I patched the drive backend to exit with an error reporting the state of use trash

$ rclone lsf drive:
2021/05/17 15:21:01 Failed to create file system for "drive:": UseTrash=true
$ RCLONE_DRIVE_USE_TRASH='false' rclone lsf drive:
2021/05/17 15:21:25 Failed to create file system for "drive:": UseTrash=false
$ RCLONE_DRIVE_USE_TRASH='false' RCLONE_CONFIG_DRIVE_USE_TRASH='true' rclone lsf drive:
2021/05/17 15:21:39 Failed to create file system for "drive:": UseTrash=false
$ RCLONE_CONFIG_DRIVE_USE_TRASH='true' rclone lsf drive:
2021/05/17 15:21:47 Failed to create file system for "drive:": UseTrash=true

Yes I think you are right - RCLONE_DRIVE_USE_TRASH and RCLONE_CONFIG_MYREMOTE_USE_TRASH should swap places.

This is because RCLONE_DRIVE_USE_TRASH is equivalent to the command line value (in fact it sets the default value for it)

The Parameters in connection strings need to be there too

So I think the correct docs should be

Parameters in connection strings, e.g. myDrive,use_trash:
Flag values as supplied on the command line, e.g. --drive-use-trash.
Command line flags set by environment vars, e.g. RCLONE_DRIVE_USE_TRASH.
Remote specific environment vars, e.g. RCLONE_CONFIG_MYREMOTE_USE_TRASH (see above).
Config file, e.g. use_trash = false.
Default values, e.g. true - these can't be changed.

It makes more sense for them to be as they were documented, but it obviously doesn't work like that! Connection strings are a far more reliable way of doing this sort of thing than RCLONE_CONFIG_MYREMOTE_USE_TRASH so hopefully that will become less common.

Ole · May 17, 2021, 4:50pm

Thanks, glad you verified and reached the same result

Not quite, then we would be missing the backend specific settings, that are possible in some situations.

An example with SKIP-LINKS would make it easier to see all the possibilities:

Parameters in connection strings, e.g. myRemote,skip_links:
Flag values as supplied on the command line, e.g. --skip-links
Command line flags set by environment vars, e.g. RCLONE_SKIP_LINKS.
Remote specific environment vars, e.g. RCLONE_CONFIG_MYREMOTE_SKIP_LINKS (see above).
Backend specific environment vars, e.g. RCLONE_LOCAL_SKIP_LINKS.
Config file, e.g. skip_links = true.
Default values, e.g. false - these can't be changed.

and then we still miss the tough part to describe why

RCLONE_DRIVE_USE_TRASH overrides RCLONE_CONFIG_MYDRIVE_USE_TRASH but
RCLONE_LOCAL_SKIP_LINKS is overridden by RCLONE_CONFIG_MYLOCAL_SKIP_LINKS

ncw · May 18, 2021, 8:19am

Ah... These are for global command line options which were missed in the first instance because their environment variables have two spellings RCLONE_SKIP_LINKS and RCLONE_LOCAL_SKIP_LINKS.

Maybe we should regularize that... Make the config variables read both the variant environment variables at the same time and get rid of this Backend specific environment vars, e.g. RCLONE_LOCAL_SKIP_LINKS. level...

Ole · May 20, 2021, 7:14pm

I hate to give up and therefore did some extra analysis...

... and found a safe way to fix all the above environment var issues

github.com/rclone/rclone

config: fixed issues with flags and options set by environment variables

rclone:master ← olefrost:fix-environment-flags

opened 11:59AM - 20 May 21 UTC

olefrost

+117 -73

#### What is the purpose of this change? Fixing issues related to the use of …environment variables. Adding debug logging of flags and options set by environment variables. Detailing the documentation of config option precedence. The changes are explained in the following sections. #### Issue1: Some env vars didn’t behave like the command line flags. _Example and impact:_ RCLONE_STATS='10s' rclone check remote: remote: --progress # Expected behaviour: rclone check remote: remote: --progress --stats=10s # Actual behaviour: rclone check remote: remote: --progress The affected flags were: --stats, --log-level, --separator, --multi-thread-streams, --rc-addr, --rc-user and --rc-pass. _Root cause and fix:_ This line https://github.com/rclone/rclone/blob/07f2f3a62e5c0461858cbd76cc04e59d49fab02e/fs/config/flags/flags.go#L24 sets the flag value directly, without updating the entire flag status such as flag.Changed. This caused unexpected behavior in code checking flag.Changed. It is fixed by replacing “flag.Value.Set(newValue)” with “flags.Set(name, newValue)”. #### Issue2: Backend env vars overruled remote specific env vars. _Example and impact:_ RCLONE_DRIVE_USE_TRASH='false' RCLONE_CONFIG_MYDRIVE_USE_TRASH='true' rclone deletefile myDrive:my-test-file # Expected result: my-test-file is recoverable in the trash folder # Actual result: my-test-file is permanently deleted (not recoverable) The issue is reproduced in [this forum post](https://forum.rclone.org/t/example-to-hit-func-oev-optionenvvars-get-key-string/24150/7). It affects all [backend options](https://rclone.org/flags/#backend-flags). _Root cause and fix:_ The line https://github.com/rclone/rclone/blob/07f2f3a62e5c0461858cbd76cc04e59d49fab02e/cmd/cmd.go#L542 was intended to create a backend flag with a possible default from an env var, but it actually sets both the value and the default from the env var. This gives the value precedence like command line values. This causes the env var to override other more specific env vars. This happens because [flags.VarPF(…)](https://github.com/rclone/rclone/blob/07f2f3a62e5c0461858cbd76cc04e59d49fab02e/fs/config/flags/flags.go#L161-L166) calls [setDefaultFromEnv(…)](https://github.com/rclone/rclone/blob/07f2f3a62e5c0461858cbd76cc04e59d49fab02e/fs/config/flags/flags.go#L14-L31) that sets both [the value](https://github.com/rclone/rclone/blob/07f2f3a62e5c0461858cbd76cc04e59d49fab02e/fs/config/flags/flags.go#L24) and [the default](https://github.com/rclone/rclone/blob/07f2f3a62e5c0461858cbd76cc04e59d49fab02e/fs/config/flags/flags.go#L29) from an env var if possible. The issue was introduced in v1.54 by this commit: https://github.com/rclone/rclone/commit/72083c65adff1d563a0f9ba92e15c2a837c420c8 The issue has been fixed by - Calling pflag.CommandLine.VarPF(…) directly without the (command line) wrapper - Creating a new func SetDefaultFromEnv(…) – only to be used when creating backend flags - Renaming func setDefaultFromEnv(…) to setValueFromEnv(…) – to avoid future mistakes. - Deleting func flags.VarPF(…) - to avoid confusion with the wrappers for command line flags #### Issue3: Backend generic env vars overruled backend specific env vars _Example and impact:_ RCLONE_SKIP_LINKS='true' RCLONE_LOCAL_SKIP_LINKS='false' rclone lsd local: # Expected result: Warnings when symlinks are skipped # Actual result: No warnings when symlinks are skipped (RCLONE_SKIP_LINKS takes precedence) The issue is reproduced in [this forum post](https://forum.rclone.org/t/example-to-hit-func-oev-optionenvvars-get-key-string/24150/3). _Root cause and fix:_ This code https://github.com/rclone/rclone/blob/07f2f3a62e5c0461858cbd76cc04e59d49fab02e/fs/fs.go#L1306-L1313 checks the generic option before the specific option. This causes the generic option to take precedence. Fixed by checking the specific (=prefixed) option first. #### Enhance1: Debug logging when options are set by environment variables. Discussed in multiple forum posts, e.g. [this ](https://forum.rclone.org/t/how-can-i-check-if-environment-variables-are-being-used/17490)and [this](https://forum.rclone.org/t/gdrive-settings-environment-variables-not-mapped/19603). Fixed by - Scanning command line and environment in [func NewConfig()](https://github.com/rclone/rclone/blob/07f2f3a62e5c0461858cbd76cc04e59d49fab02e/fs/config.go#L137-L141) to set LogLevelDebug preliminarily before flags are being initialised and possibly read from the environment. - Adding and streamlining debug logging whenever environment variables are being read. - Disabling a very frequent [`rc` debug message](https://github.com/rclone/rclone/blob/07f2f3a62e5c0461858cbd76cc04e59d49fab02e/fs/rc/registry.go#L48) during initialisation. - Adding tips about debug logging of environment variables to the documentation. #### Enhance2: Improved the documentation of config option precedence. Inspired by [this forum post](https://forum.rclone.org/t/example-to-hit-func-oev-optionenvvars-get-key-string/24150/9). #### Checklist - [x] I have read the [contribution guidelines](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#submitting-a-pull-request). - [x] I have added tests for all changes in this PR if appropriate. - [x] I have added documentation for the changes if appropriate. - [x] All commit messages are in [house style](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#commit-messages). - [x] I'm done, this Pull Request is ready for review :-)

ncw · May 21, 2021, 8:09am

Well done! Will reply on the PR.

system · July 21, 2021, 4:10am

This topic was automatically closed 60 days after the last reply. New replies are no longer allowed.