Skip to content

Generate zoslib env hook#236

Merged
IgorTodorovskiIBM merged 5 commits intomainfrom
zoslib_env
Mar 28, 2023
Merged

Generate zoslib env hook#236
IgorTodorovskiIBM merged 5 commits intomainfrom
zoslib_env

Conversation

@IgorTodorovskiIBM
Copy link
Copy Markdown
Member

@IgorTodorovskiIBM IgorTodorovskiIBM commented Mar 17, 2023

Works with zoslib PR: ibmruntimes/zoslib#13.

For git, this allows it to run without having to source the .env:

zopen_append_to_zoslib_env() {
cat <<EOF
GIT_TEMPLATE_DIR|set|PROJECT_ROOT/share/git-core/templates
GIT_EXEC_PATH|set|PROJECT_ROOT/libexec/git-core
GIT_SSL_CAINFO|set|PROJECT_ROOT/cacert.pem
EOF
}

Copy link
Copy Markdown
Collaborator

@MikeFultonDev MikeFultonDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we discussed this and the recommendation is to go with a solution that binds this information into the programs rather than using a side file

@IgorTodorovskiIBM IgorTodorovskiIBM changed the title Add .zoslib_env files Generate zoslib env hook Mar 23, 2023
@IgorTodorovskiIBM
Copy link
Copy Markdown
Member Author

we discussed this and the recommendation is to go with a solution that binds this information into the programs rather than using a side file

The latest commits address this. The related zoslib PR is: ibmruntimes/zoslib#13

Comment thread bin/lib/zopen-build
cd -
}

generateZOSLIBHooks()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment block to describe what this is doing?
For the C code, does this pre-req CLANG or will other compilers work?

Comment thread bin/lib/zopen-build
echo "$zoslib_env" | while read line; do
var=$(echo $line | cut -d '|' -f 1)
action=$(echo $line | cut -d '|' -f 2)
value=$(echo $line | cut -d '|' -f 3)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should describe the format of the environment 'hooks' - what are the valid values (PREPEND and APPEND ?) What are the supported 'well-known' variables (just PROJECT_ROOT ?) Why is PATH required? if the program isn't in the PATH then they must have explicitly executed it via relative or absolute specification in which case, it can be determined? Why is LIBPATH required? Can't it be determined from the location of the program (e.g. <program absolute location>/../lib ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should check for var/action/value being valid and that the line is a valid line

Comment thread bin/lib/zopen-build Outdated
// Substitute PROJECT_ROOT with root_dir
pos = strstr(value_str, "PROJECT_ROOT");
if (pos != NULL) {
int len = pos - value_str;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe something more descriptive than len? This is the length of the prefix before the PROJECT_ROOT correct?

Comment thread bin/lib/zopen-build Outdated
int len = pos - value_str;
strncat(envar_value, value_str, len);
strcat(envar_value, root_dir);
strcat(envar_value, pos + strlen("PROJECT_ROOT"));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since PROJECT_ROOT is a hard-coded string, why not have a macro with that string and then you can have a PROJECT_ROOT_LEN that is just sizeof(PROJECT_ROOT)-1 ? That saves a strlen call...

Comment thread bin/lib/zopen-build
#endif
if (!getenv("$var")) {
if (setenv("$var", envar_value, 1) != 0) {
fprintf(stderr, "Error: Setting environment variable %s=%s\n", "$var", envar_value);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be a hard failure if the env var can't be set?

Comment thread bin/lib/zopen-build
zz
elif [ "$action" = "prepend" ]; then
cat <<zz >>$output
value_str = "$value";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar comments as for set

Comment thread bin/lib/zopen-build
echo "}" >> $output

if [ "$CC" = "xlclang" ]; then
extraOptions="-qexportall"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does xlclang need exportall? Why doesn't clang? Does everything have to be exported, or could you use a #pragma export directive ?

Copy link
Copy Markdown
Collaborator

@MikeFultonDev MikeFultonDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@IgorTodorovskiIBM IgorTodorovskiIBM merged commit 271087e into main Mar 28, 2023
@MikeFultonDev MikeFultonDev deleted the zoslib_env branch March 31, 2023 03:09
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 this pull request may close these issues.

2 participants