uci merge-import doesn't work unless complete path is specified

[friendly-bits]

To reproduce the issue:

# create test files
printf '%s\n%s\n' "config conf1 'conf1_name'" "option opt1_name 'opt1_val'" > /etc/config/test1
printf '%s\n%s\n' "config conf2 'conf2_name'" "option opt2_name 'opt2_val'" > /etc/config/test2
# use 1st test file as source
cp  /etc/config/test1  /etc/config/test-merge
# merge 2nd test file
uci -m import test-merge < /etc/config/test2
cat /etc/config/test-merge 

Expected output:

config conf1 'conf1_name'
	option opt1_name 'opt1_val'

config conf2 'conf2_name'
	option opt2_name 'opt2_val'

Actual output:

config conf1 'conf1_name'
option opt1_name 'opt1_val'

uci import (without -m) works fine:

cp  /etc/config/test1  /etc/config/test-merge
uci import test-merge < /etc/config/test2
cat /etc/config/test-merge 

Output:

config conf2 'conf2_name'
	option opt2_name 'opt2_val'

uci -m import works fine when complete path is specified:

cp  /etc/config/test1  /etc/config/test-merge
uci -m import /etc/config/test-merge < /etc/config/test2

cat /etc/config/test-merge

Output:

config conf1 'conf1_name'
	option opt1_name 'opt1_val'

config conf2 'conf2_name'
	option opt2_name 'opt2_val'

Tested on OpenWrt 24.10.5

[bluewavenet]

@friendly-bits
You need to do a uci commit to write the config file to flash.

Try:

# create test files
printf '%s\n%s\n' "config conf1 'conf1_name'" "option opt1_name 'opt1_val'" > /etc/config/test1
printf '%s\n%s\n' "config conf2 'conf2_name'" "option opt2_name 'opt2_val'" > /etc/config/test2
# use 1st test file as source
cp  /etc/config/test1  /etc/config/test-merge
# merge 2nd test file
uci -m import test-merge < /etc/config/test2
uci commit test-merge
cat /etc/config/test-merge 

[friendly-bits]

@bluewavenet Running uci commit does work, but this seems inconsistent. As the other 2 code snippets in the report show, uci -m import <complete_path> updates the file immediately (without uci commit). uci import <config_name> (without -m) does the same.

[bluewavenet]

@friendly-bits
If anything is wrong, it is giving the complete path forces a write to the file.
The code logic in uci is probably "if full path specified - don't keep in memory" or something similar (but this is undocumented as far as I can see).

Anyway, writing the file only on commit is the intended default. This allows dynamic, runtime config changes without causing flash wear.

[friendly-bits]

writing the file only on commit is the intended default

How do you know what's the intended default? The command in its simplest form - uci import <config_name> - writes the file immediately. From this, I assumed that the import command does not use the 'config buffer'.

[bluewavenet]

@friendly-bits

When you do a direct write to /etc/config things get a bit broken.
Lets use your test files:

mkdir -p /tmp/ucitest
# create test files
printf '%s\n%s\n' "config conf1 'conf1_name'" "option opt1_name 'opt1_val'" > /tmp/ucitest/test1
printf '%s\n%s\n' "config conf2 'conf2_name'" "option opt2_name 'opt2_val'" > /tmp/ucitest/test2

I'll do a test.

First, make sure there is nothing left from previous testing:

root@meshnode-8ecb:/tmp/ucitest# rm /etc/config/test-merge
root@meshnode-8ecb:/tmp/ucitest# uci show test-merge
uci: Entry not found
root@meshnode-8ecb:/tmp/ucitest# cat /etc/config/test-merge
cat: can't open '/etc/config/test-merge': No such file or directory
root@meshnode-8ecb:/tmp/ucitest# 

Now instead of copying the first file, I will import it - the test-merge config file does not exist yet.

root@meshnode-8ecb:/tmp/ucitest# uci import test-merge < test1
root@meshnode-8ecb:/tmp/ucitest# cat /etc/config/test-merge

config conf1 'conf1_name'
	option opt1_name 'opt1_val'

root@meshnode-8ecb:/tmp/ucitest#

The target config file did not exist so uci created it and it now contains the test1 config in sync with the "config buffer".

Now lets merge test2:

root@meshnode-8ecb:/tmp/ucitest# uci -m import test-merge < test2
root@meshnode-8ecb:/tmp/ucitest# 
root@meshnode-8ecb:/tmp/ucitest# uci show test-merge
test-merge.conf1_name=conf1
test-merge.conf1_name.opt1_name='opt1_val'
test-merge.conf2_name=conf2
test-merge.conf2_name.opt2_name='opt2_val'
root@meshnode-8ecb:/tmp/ucitest# 
root@meshnode-8ecb:/tmp/ucitest# cat /etc/config/test-merge

config conf1 'conf1_name'
	option opt1_name 'opt1_val'

root@meshnode-8ecb:/tmp/ucitest# 

So now, test2 has been merged with the config buffer. But we have not committed yet so the /etc/config/test-merge file is so far unchanged.

Lets commit:

root@meshnode-8ecb:/tmp/ucitest# 
root@meshnode-8ecb:/tmp/ucitest# uci commit test-merge
root@meshnode-8ecb:/tmp/ucitest# 
root@meshnode-8ecb:/tmp/ucitest# cat /etc/config/test-merge

config conf1 'conf1_name'
	option opt1_name 'opt1_val'

config conf2 'conf2_name'
	option opt2_name 'opt2_val'

root@meshnode-8ecb:/tmp/ucitest# 

The uci commit rewrote the /etc/config/test-merge file, bringing it back into sync with the config buffer.

This is how it is supposed to work.

How do you know what's the intended default?

I don't "know" in that I have not studied the details of the code, but this is how it has always worked.

Strictly, config files should only be touched by uci as directly writing to them messes with the sync functionality uci depends on to minimise config writes to flash.

However, people edit config files willy-nilly and mostly get away with it until someone/somepackage tries to do it properly using uci commands.

[friendly-bits]

Strictly, config files should only be touched by uci as directly writing to them messes with the sync functionality uci depends on to minimise config writes to flash.

I don't think there is a point in discussing how it "should" work since neither of us has any say in this, and the documentation doesn't clarify things. This issue is about how it "does" work.

As your own (and my) testing shows, the uci import <config_name> command writes to file immediately. As my testing shows, the uci -m import <complete_path> command writes to file immediately. The uci -m import <config_name> command is an outlier in this context, creating a situation of inconsistent API.

I'm not sure what's the best way forward from here. Ideally, the API should be made consistent, either by changing the behavior of uci -m import <config_name> to perform immediate writes, or by changing the behavior of uci -m import <complete_path> and uci import <config_name> to not perform immediate writes. (I understand that you have your preference but as mentioned above, the discussion of "how this should work" is pointless IMO). The problem of course is that if this inconsistency is a bug then it's also a feature which some projects may be relying on. At least qosmate expects the uci import <config_name> to write immediately.

So it's up to the devs how to handle this, I just wanted to report the issue. Hopefully, at the very least the behavior will get documented.

[bluewavenet]

@friendly-bits
We will have to agree to disagree.

What you are seeing is down to directly writing to a config file rather than using uci to make changes or create it.

In my opinion, documentation should clearly say "Edit or Create config files directly AT YOUR OWN RISK"

I did not give a guess of "how this should work", rather I stated "this is how it is supposed to work", meaning this is, in practice, how it DOES work.

As your own (and my) testing shows, the uci import <config_name> command writes to file immediately.

Yes, but ONLY if the /etc/config file does not already exist (as in your example of qosmate).

As my testing shows, the uci -m import <complete_path> command writes to file immediately. The uci -m import <config_name> command is an outlier in this context, creating a situation of inconsistent API.

With full path, I get:

root@meshnode-8ecb:/tmp/ucitest# uci import /etc/config/test1-merge < /tmp/ucitest/test1
uci: I/O error
root@meshnode-8ecb:/tmp/ucitest# 
root@meshnode-8ecb:/tmp/ucitest# 
root@meshnode-8ecb:/tmp/ucitest# uci -m import /etc/config/test1-merge < /tmp/ucitest/test1
root@meshnode-8ecb:/tmp/ucitest# cat /etc/config/test1_merge
cat: can't open '/etc/config/test1_merge': No such file or directory
root@meshnode-8ecb:/tmp/ucitest# 

So for me, it works as expected (by me, and as it always has, for me). There is no inconsistency.
BUT if you manually create a file (eg by copying to /etc/config from somewhere, things can and do get out of sync and break.

[friendly-bits]

root@meshnode-8ecb:/tmp/ucitest# uci -m import /etc/config/test1-merge < /tmp/ucitest/test1
root@meshnode-8ecb:/tmp/ucitest# cat /etc/config/test1_merge

Yes, it throws an error if the file doesn't exist yet. If it does exist then this command immediately overwrites it:

oot@OpenWrt:~# uci -m import /etc/config/test-merge < /etc/config/test1
uci: I/O error
root@OpenWrt:~# uci import test-merge < /etc/config/test1
root@OpenWrt:~# cat /etc/config/test-merge

config conf1 'conf1_name'
	option opt1_name 'opt1_val'

root@OpenWrt:~# uci -m import /etc/config/test-merge < /etc/config/test2
root@OpenWrt:~# cat /etc/config/test-merge

config conf1 'conf1_name'
	option opt1_name 'opt1_val'

config conf2 'conf2_name'
	option opt2_name 'opt2_val'

Yes, but ONLY if the /etc/config file does not already exist

Above example shows that uci -m import <complete_path> overwrites an already-existing config file (which was properly imported before via a UCI command). Same for uci import <config_name>:

root@OpenWrt:~# rm -f /etc/config/test-merge
root@OpenWrt:~# uci import test-merge < /etc/config/test1
root@OpenWrt:~# cat /etc/config/test-merge

config conf1 'conf1_name'
	option opt1_name 'opt1_val'

root@OpenWrt:~# uci import test-merge < /etc/config/test2
root@OpenWrt:~# cat /etc/config/test-merge

config conf2 'conf2_name'
	option opt2_name 'opt2_val'

[bluewavenet]

@friendly-bits
This is interesting.

The syntax is:

root@meshnode-8ecb:/etc/config# uci
Usage: uci [<options>] <command> [<arguments>]

Commands:
	batch
	export     [<config>]
	

So uci export [<config>] where [<config>] is the config name eg test-merge in our example.

I'll go through this again for clarity... then elaborate what I am getting at.
Now:
uci import test-merge < /etc/config/test1
puts the new config into the "uci buffer" and because the file does not exist, uci creates it, leaving the buffer and file in sync.

Next:
uci -m import test-merge < test2
This merges test 2, only in the buffer - consistent with all other commands eg uci set ... etc.
But the file still only contains what was imported from /etc/config/test1

Finally, doing uci commit test-merge writes the buffer to the file. Again, the result is fully consistent with other uci commands.

What I am getting at:
But if you use the file path? This is not documented - the syntax shows only the use of the config name.

It seems if we do use the file path, the buffer is bypassed and the config file is overwritten.

Yes, this is undocumented, but also potentially useful.

Is the bug in uci just the fact this is not documented in the "help" output?

Thanks for invoking some deep thought on the subject. Trying to read the code is a horrible task as it is, in my opinion, not presented in an easily digestible fashion... I guess I will pour a glass of something stronger than coffee and try again 🥃

[friendly-bits]

uci import test-merge < /etc/config/test1
puts the new config into the "uci buffer" and because the file does not exist, uci creates it, leaving the buffer and file in sync.

No. uci import <config> < <file_path> always immediately overwrites the config file and does not store anything in the config buffer. This is specifically shown in the first post of this issue, and in the post you were replying to. I'll demonstrate this again:

root@OpenWrt:~# uci import test-merge < /etc/config/test1
root@OpenWrt:~# cat /etc/config/test-merge 

config conf1 'conf1_name'
	option opt1_name 'opt1_val'

root@OpenWrt:~# uci import test-merge < /etc/config/test2
root@OpenWrt:~# cat /etc/config/test-merge 

config conf2 'conf2_name'
	option opt2_name 'opt2_val'

Is the bug in uci just the fact this is not documented in the "help" output?

I don't know whether there is a bug. To me, the issue is that the uci import API is inconsistent. Maybe it's by design. If it's not by design then it's a bug. Either way, it should be documented. Could be in the help output, could be in the wiki, or (best) both.

[bluewavenet]

@friendly-bits

No. uci import < <file_path> always immediately overwrites the config file and does not store anything in the config buffer.

If the buffer and file are not in sync, it does write the file. If you copy the file from somewhere else, as you were doing, then the buffer and file will of course be out of sync. The buffer will be empty.

But if you create the file using uci import, The buffer is filled AND the file is written.

Both can be verified with uci export command, which outputs the buffer.
The uci changes command gives a good insight of the process, comparing buffer and file.

I am just trying to get it totally clear in my mind - I can then at least update the wiki.

Looking at the code, it is not easy to follow, but yes it differentiates between config name and config file_path but it all seems to be obfuscated C code to my eyes. Sorry to those that wrote it, but some clear structure with useful comments would be nice. Maybe for someone writing C all day every day it would be clear what is going on..

[wififreedom]

FYI in delta.c, function uci_save has a comment that says:

/*
 * if the config file was outside of the /etc/config path,
 * don't save the delta to a file, update the real file
 * directly.
 * does not modify the uci_package pointer
 */

Disclaimer: I have not tested this. This is just what it says.