Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes in PGENV_INITDB_OPTIONS (config-file) are not applied #56

Closed
17bob17 opened this issue Sep 12, 2022 · 19 comments · May be fixed by #57
Closed

Changes in PGENV_INITDB_OPTIONS (config-file) are not applied #56

17bob17 opened this issue Sep 12, 2022 · 19 comments · May be fixed by #57

Comments

@17bob17
Copy link

17bob17 commented Sep 12, 2022

Hello,

first of all thank you for the great tool. I am pretty new to the tool and make my first steps with it.

My environment is:

OS: Redhat Linux 8.6 x64
Kernel: 4.18.0-372.19.1.el8_6.x86_64
Shell: Bash
User: postgres
pgenv-Version: 1.3.1

In default.conf (under directory config) I changed Initdb-Flags to:
declare -a PGENV_INITDB_OPTIONS=([0]="-U" [1]="postgres" [2]="--locale" [3]="de_DE.UTF-8" [4]="--encoding" [5]="UNICODE" [6]="--data-checksums")

Unfortuntely the new parameters didn't make it to the execution. Instead the standard values in the pgenv-script are used.

Changes of initdb-parameters will be applied if I change PGENV_INITDB_OPTIONS in the pgenv-script directly.

But I think the better solution would be the expected way with the change in the config-file.

The same problem seems to occur with changes in PGENV_CONFIGURE_OPTIONS.

I tried to debug the situation a little bit and it seems to me that the changes in the config-file are visible (exported) as long as I am in the function "pgenv_configuration_load" but get lost as soon as I leave the function.

But maybe it's a mistake on my side and I'm doing something wrong.

Best regards
Kai

@fluca1978
Copy link
Collaborator

This is somehow related to #52 and #54: if you change the default.conf to use declare -g instead of declare -a you should get the correct behavior.
Somehow variables sourced into the pgenv_configuration_load are automatically lexycally scoped to such function.

One possible solution could be to change the loading mechanics: since pgenv_configuration_load finds out the correct file to load, we could source such file within every piece a configuration is needed. I have to test this, and is clearly not an elegant solution.

Another, possibly better, approach could be to use declare -g on non Apple machines, but I don't have a mac to test how this works on OSX. I suspect 'declare -aon mac (#54) works fine asdeclare -g` on other Bash implementations.

@17bob17
Copy link
Author

17bob17 commented Sep 12, 2022

Thank you for your quick response and sorry - I didn't see the two related items.
I tried a build/use with declare -g and declare -ag but didn't get consistent results so far.
WIll start a new try tomorrow.

@fluca1978
Copy link
Collaborator

declare -g seems to work on my Linux Bash 5.0.17

@17bob17
Copy link
Author

17bob17 commented Sep 13, 2022

you're right, declare -g is working.
What confused me is the following:
I did a "pgenv build" and then a "pgenv use". My default config file has a declare -g entry for Configure-Options and InitDB-Options. The build was o.k. with the Configure-Options specified. But then it seems that the newly created, version-specific config file now has entries for Config-Options and InitDB-Options with "declare -ax" instead of "declare -g". Then, when I do a "pgenv use ", this version-config file is used and with declare -ax the InitDB-Options doesn't get exported.
If I remove the version-specific config file before doing a "pgenv use", the standard file seems ti get used with the declare -g entry and everything is fine.

@theory
Copy link
Owner

theory commented Sep 13, 2022

On my Mac:

bash -c "help declare"
declare: declare [-afFirtx] [-p] [name[=value] ...]
    Declare variables and/or give them attributes.  If no NAMEs are
    given, then display the values of variables instead.  The -p option
    will display the attributes and values of each NAME.
    
    The flags are:
    
      -a	to make NAMEs arrays (if supported)
      -f	to select from among function names only
      -F	to display function names (and line number and source file name if
    	debugging) without definitions
      -i	to make NAMEs have the `integer' attribute
      -r	to make NAMEs readonly
      -t	to make NAMEs have the `trace' attribute
      -x	to make NAMEs export
    
    Variables with the integer attribute have arithmetic evaluation (see
    `let') done when the variable is assigned to.
    
    When displaying values of variables, -f displays a function's name
    and definition.  The -F option restricts the display to function
    name only.
    
    Using `+' instead of `-' turns off the given attribute instead.  When
    used in a function, makes NAMEs local, as with the `local' command.

@fluca1978
Copy link
Collaborator

But then it seems that the newly created, version-specific config file now has entries for Config-Options and InitDB-Options with "declare -ax" instead of "declare -g".

Clearly, the newly created configuration file still has the -a option and not the -g. I was simply suggesting to manually edit your configuration file (either default, or version specific) to test if it worked.

@theory should we test for uname -s and use declare -ag or declare -ax?

@17bob17
Copy link
Author

17bob17 commented Sep 14, 2022

Ah, thank you for your clarification. I thought on a new build the default-config is somehow copied as version-config-file. Now I understand that this is not the case.

@fluca1978
Copy link
Collaborator

Ah, thank you for your clarification. I thought on a new build the default-config is somehow copied as version-config-file. Now I understand that this is not the case.

To be more clear: the default config is used for a new build as a starting point, then the config is dumped (not copied) to a version specific file. The problem is, in fact, in the dumping strategy, that applies declare -a while it sould append -x (or -g) flag too.

I wonder why Bash has such a different flag on OSX than on other Linux systems.

@theory
Copy link
Owner

theory commented Sep 14, 2022

Probably because the version of bash on macOS is super old:

$ /bin/bash --version   
GNU bash, version 3.2.57(1)-release (arm64-apple-darwin21)
Copyright (C) 2007 Free Software Foundation, Inc.

Lots of people use the Homebrew bash, instead:

$ /opt/homebrew/bin/bash --version
GNU bash, version 5.1.16(1)-release (aarch64-apple-darwin21.1.0)
Copyright (C) 2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

It includes -g, but we can't count on it. Super lame. I wonder if -x will do what we want, though.

@fluca1978
Copy link
Collaborator

I suspect the problem is arising from an old configuration file.
In fact, according to the 76645e8 commit, each variable is explicitly exported by a well supported EXPORT command.
This means we don't have to deal with declare flags.

However, this is true for newly written configuration files, not for migrated (pgenv config migrate) files.

@17bob17 can you do the following and see if this works?

  1. pgenv config write <your postgresql version>
  2. edit the file generated and append your settings
  3. do a fresh build/start and see if the options are kept.

If that works, you should note that in your configuration specific version file you have an EXPORT after each declare line.
Such export should be missing by your default configuration file.

@theory if my guess is correct, we should at least warn the user that he has a default configuration file "older" than the one expected by the application. This means we should store and check a "versioning" into the configuration files...

@17bob17
Copy link
Author

17bob17 commented Sep 16, 2022

I tried it but unfortunately, it didn't work. I did the following (here with version 13.8):

pgenv clear
pgenv remove 13.8 (after that, no pgsql-13.8 folder and no 13.8.conf anymore)
pgenv config write 13.8
Had those entries in 13.8.conf:

Configure flags, including PL languages but without --prefix
declare -ax PGENV_CONFIGURE_OPTIONS=([0]="--with-icu" [1]="--with-pam" [2]="--with-perl" [3]="--with-ldap" [4]="--with-tcl" [5]="--with-openssl" [6]="--with-systemd" [7]="--with-libxml" [8]="--with-libxslt" [9]="--with-selinux" [10]="--with-gssapi")
export PGENV_CONFIGURE_OPTIONS

Initdb flags
declare -ax PGENV_INITDB_OPTIONS=([0]="-U" [1]="postgres" [2]="--locale" [3]="de_DE.UTF-8" [4]="--encoding" [5]="UNICODE" [6]="--data-checksums")
export PGENV_INITDB_OPTIONS

pgenv build 13.8 - 13.8.conf newly written and now I have:

Configure flags, including PL languages but without --prefix
declare -a PGENV_CONFIGURE_OPTIONS()
export PGENV_CONFIGURE_OPTIONS

Initdb flags
declare -a PGENV_INITDB_OPTIONS=([0]="-U" [1]="postgres" [2]="--locale" [3]="en_US.UTF-8" [4]="--encoding" [5]="UNICODE")
export PGENV_INITDB_OPTIONS

pgenv use 13.8

pg_configure shows:
CONFIGURE = '--prefix=/var/lib/pgsql/.pgenv/pgsql-13.8'

initdb prepares the cluster with en_US.UTF8 and without checksums

Some observations:

"pgenv config write" seems to duplicate the entries in default.conf but with "declare -ax" instead of "declare -g".
declare -ax is not working in my environment although the variables are also exported. "declare -g" is working fine.
"pgenv build 13.8" with newly created 13.8.conf seems to ignore my default.conf in respect to parameter values and also ignores or can't use the values in 13.8.conf. In the end I get a more or less default installation.

Hope that helps a little bit.

@fluca1978
Copy link
Collaborator

After trying it a little more:

  • declare -ax is not working;
  • EXPORT seems to be ignored on arrays
  • loaded variables trhu source are locally scoped to pgenv_configuration_load, as @17bob17 initially reported
  • doing an explicit EXPORT of a variable in pgenv_configuration_load is not working too

But then came the idea: if we remove the declare keyword so that in your configuration file you have:

PGENV_INITDB_OPTIONS=([0]="-U" [1]="postgres" [2]="--locale" [3]="de_DE.UTF-8" [4]="--encoding" [5]="UNICODE" [6]="--data-checksums")

it works, even without an explicit EXPORT

@17bob17 can you please edit once again you configuration file, removing the declare -a and declare -ax everywhere and see if works?

@17bob17
Copy link
Author

17bob17 commented Sep 16, 2022

Yes, you are right, all this is working:

declare -g PGENV_INITDB_OPTIONS=(.....)
or
PGENV_INITDB_OPTIONS=(.....)
export PGENV_INITDB_OPTIONS
or
PGENV_INITDB_OPTIONS=(.....) [wothout export]

The only thing is that I have to adjust the version-specific config-file between "build" and "use". But probably you know that. In my opinion it would be nice if the use-phase would take its config from the initial default-config. But maybe that is only good for my use case and could break other things.

fluca1978 added a commit to fluca1978/pgenv that referenced this issue Sep 17, 2022
`declare -a` is used for arrays when `declare -p` dumps the
configuration. However when `pgenv_configuration_read` gets back the
configuration, variables becomes locally scoped (see `bash -c "help
declare"). One solution could be to use `declare -g` to make variables
globals, but this does not works on OSX.
Removing `declare -a` from arrays seems to make the variable global
even if no `EXPORT` is issued. As a possible compatibility statement,
the `EXPORT` after each variable is left in place.

Close theory#56
@fluca1978
Copy link
Collaborator

@17bob17 could you please check out #57 and see if it works fine for you? You need to nuke your config directory (make a backup of it) and write configuration from scratch.

If this works, I'm going to merge and release.
@theory could you please check you too #57 works fine on OSX?

@17bob17
Copy link
Author

17bob17 commented Sep 18, 2022

Hi,
I tested it and as long as there is no "declare" in the config-file it's working fine. But also I found that after a build or a rebuild the config-file is generated with "declare -ax". For example:

pgenv config write --> generates a default config file without declare-clause
changed default config file with my INITDB-/CONFIGURE-Options
pgenv build 13.8 --> generates a 13.8.conf with "declare -ax"-clauses.
pgenv use 13.8
RESULT: CONFIGURE-Options are used because of the default file.
InitDB-Options didn't get used because of "declare -ax" in 13.8.conf

@fluca1978
Copy link
Collaborator

My fault, I've updated #57 right now and it works now on my system, i.e., it does not include any declare anymore even after a build.
@17bob17 cold you kindly try it again after updating?

@17bob17
Copy link
Author

17bob17 commented Sep 19, 2022

Tested it with the updated version using build and rebuild in varying constellations and now it works perfectly.

Thank you very much.

@fluca1978
Copy link
Collaborator

Great!

@theory ok to merge and push a minor version?

@theory
Copy link
Owner

theory commented Sep 20, 2022

Yes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants