search

DannyBen / bashly

Bash command line framework and CLI generator
https://bashly.dannyb.co
MIT License
2.07k stars 85 forks source link

Bug in ini library #556

Open mcblum opened 2 weeks ago

mcblum commented 2 weeks ago

Bashly Version

Latest Ruby Gem

Description

What I'm seeing is that sometimes values are added or deleted from the wrong section. I was banging my head against the wall trying to figure out what was going on and then I ran a test operation and asked it to delete tenant.DATA_PATH from one section but it, in fact, deleted it from another. This has caused some serious issues.

Also, sometimes the ini file is just blank altogether, which I really don't understand.

Contents of bashly.yml

No response

Reproduction Steps

Basically I'm looping through a bunch of tenant folders, reading their INI files, and sometimes updating values. The updates are frequently getting messed up.

Actual Behavior

Keys are randomly deleted from the ini file, sometimes the ini file is written blank.

Expected Behavior

That loading the INI and changing a value would work as expected

DannyBen commented 2 weeks ago

Without a minimal, simple way to reproduce the problem, there is not much I can do.

mcblum commented 2 weeks ago

I'm working on it right now! I would guess that means this isn't something that people are reporting often?

DannyBen commented 2 weeks ago

No reports. You can also take a look at the code for the INI library, it really doesn't do much.

https://github.com/DannyBen/bashly/blob/c0fd6f25b25065375638e03e5d61804cef4563ff/lib/bashly/libraries/ini/ini.sh#L56-L88

Nsomnia commented 2 weeks ago

Do a vs line by line step debug, bang in some sleep/echo/printf commands in bashly, bashdb/similar, or whatever you might be using as libs/src with related ideas, to figure out when and why it happens?

DannyBen commented 2 weeks ago

I am not following.

If it is a bug in the INI library in bashly, it should be easily reproducible. If the code to reproduce is it complicated, you need to uncomplicate it, and be sure it is a bug in the INI library provided with bashly, and not elsewhere in your script.

If it is inconsistent, then I suspect a race condition.

Keep in mind that the code libraries provided with bashly are provided as a basis for you to build on. It is not the primary purpose of bashly to provide you with bash snippets. That said, if there is an obvious bug with any of the provided libraries, I would like to have them fixed.

mcblum commented 2 weeks ago

If it is inconsistent, then I suspect a race condition.

This is exactly what I was thinking, but I can't find where that would be happening. I'm finding it hard to reproduce because IRL there are a lot of variables, so it's a lot of stuff to fake. The general process for us is

I suspect that's where the issue might be. I also hear what you're saying about it not being a Bashly thing, and that makes sense.

DannyBen commented 2 weeks ago

As you can see from the first line in the ini_save() function:

https://github.com/DannyBen/bashly/blob/c0fd6f25b25065375638e03e5d61804cef4563ff/lib/bashly/libraries/ini/ini.sh#L56-L57

it relies on a global variable - to set/get values.

This means that if it is called more than once at the same time, you will see issues. It was not designed for it. See if there is any chance your code calls it with overlapping calls.

Another thing it was not necessarily designed for - and perhaps this CAN be reproduced - is to handle multiple INI files at once - even if it is one after the other. It was designed to be more of a function to let you read your own INI config file. So perhaps this is a direction where we can try and reproduce - just do a loop of read/writing - I suspect that the global variable may remain "contaminated" in some scenarios.

mcblum commented 2 weeks ago

When you say with overlapping calls, do you mean within the same function run or two things running the script at the same time? I think you're absolutely correct about it being an issue with multiple ini files. I added some code to work around this by, essentially, clearing the ini array as part of a reset function, but I would guess that's where the issue is, too.

I'll hack on this to see what I can do with it to expand/improve upon it. I think maybe I'll attempt to add something like:

config_load "$FILEPATH" "$NAME"

If filepath and name are set, then it would create an array called name_ini instead of just ini and then you could perform operations on a specify an array and I wouldn't have to clear things out all the time.

DannyBen commented 2 weeks ago

See if this helps:

Testing multiple INI files loaded in the same script causes an issue now. Here is the reproduction:

ini_load "test.ini"

for key in $(ini_keys); do
  echo "- $key = ${ini[$key]}";
done

# this is the fix.
# without it, the next piece of code will show entries from both INI files.
unset ini   

ini_load "test2.ini"

for key in $(ini_keys); do
  echo "- $key = ${ini[$key]}";
done

This will "blend" all configuration from all loaded INI files, unless you call "unset ini" as I did in the script. Could this be the same issue you see?

Of course, you can add the "unset ini" line inside the load_ini() function. If this is indeed your issue, we can probably safely add it to the library itself.

DannyBen commented 2 weeks ago

In general, you should consider this:

The few bashly libraries you can add with bashly add are added to your folder by design. The reason being, that you should treat them almost as if they are your own code.

In the case you discovered - it seems like adding unset ini, or providing means for specifying the name of the variable (or both) - might be of interest to others.

So - I am waiting to hear if adding unset ini inside the ini_load function seems to be solving your issue. If so, we can proceed.