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.

Sign up for GitHub

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

Jump to bottom

DAOS-14981 gurt: restore d_getenv_int undefined symbol #13622

Merged
merged 5 commits into from
Jan 19, 2024
Merged

DAOS-14981 gurt: restore d_getenv_int undefined symbol #13622

merged 5 commits into from
Jan 19, 2024

Conversation

knard38
Copy link
Contributor

@knard38 knard38 commented Jan 17, 2024 •
edited
Loading

Description

Restore missing plain function d_getenv_int() to fix missing symbol with libdaos.

Before requesting gatekeeper:

  • Two review approvals and any prior change requests have been resolved.
  • Testing is complete and all tests passed or there is a reason documented in the PR why it should be force landed and forced-landing tag is set.
  • Features: (or Test-tag*) commit pragma was used or there is a reason documented that there are no appropriate tags for this PR.
  • Commit messages follows the guidelines outlined here.
  • Any tests skipped by the ticket being addressed have been run and passed in the PR.

Gatekeeper:

  • You are the appropriate gatekeeper to be landing the patch.
  • The PR has 2 reviews by people familiar with the code, including appropriate watchers.
  • Githooks were used. If not, request that user install them and check copyright dates.
  • Checkpatch issues are resolved. Pay particular attention to ones that will show up on future PRs.
  • All builds have passed. Check non-required builds for any new compiler warnings.
  • Sufficient testing is done. Check feature pragmas and test tags and that tests skipped for the ticket are run and now pass with the changes.
  • If applicable, the PR has addressed any potential version compatibility issues.
  • Check the target branch. If it is master branch, should the PR go to a feature branch? If it is a release branch, does it have merge approval in the JIRA ticket.
  • Extra checks if forced landing is requested
    • Review comments are sufficiently resolved, particularly by prior reviewers that requested changes.
    • No new NLT or valgrind warnings. Check the classic view.
    • Quick-build or Quick-functional is not used.
  • Fix the commit message upon landing. Check the standard here. Edit it to create a single commit. If necessary, ask submitter for a new summary.

@kanard38
DAOS-14981 gurt: restore d_getenv_int undefined symbol
b1101e6 
Restore missing plain function d_getenv_int() to fix missing symbol with
libdaos.

Features: mpiio dfuse
Required-githooks: true
Signed-off-by: Cedric Koch-Hofer <[email protected]>
@knard38 knard38 self-assigned this Jan 17, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 17, 2024 •
edited
Loading

Bug-tracker data:
Ticket title is 'dfuse/daos_build.py:DaosBuild.test_dfuse_daos_build_wt_il - /usr/lib64/libdaos_common.so: undefined symbol: d_getenv_int'
Status is 'In Review'
Labels: 'ci_impact,daily_test,release/2.4,request_for_2.6,triaged'
https://daosio.atlassian.net/browse/DAOS-14981

@knard38 knard38 marked this pull request as ready for review January 17, 2024 09:16
ashleypittman
ashleypittman reviewed Jan 17, 2024
View reviewed changes
src/include/gurt/common.h Outdated
@@ -586,6 +586,9 @@ int
d_getenv_bool(const char *name, bool *bool_val);
int
d_getenv_char(const char *name, char *char_val);
/* DAOS-14981 XXX d_getenv_int() is deprecated, please use d_getenv_uint() */
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a compiler intrinsic you should use here instead which will make the compiler warn if the function is used.

src/include/gurt/common.h Outdated
Comment on lines 590 to 591
int
d_getenv_int(const char *name, unsigned int *uint_val);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not at my desk so can't try it but this should work.

Suggested change
int
d_getenv_int(const char *name, unsigned int *uint_val);
int
d_getenv_int(const char *name, unsigned int *uint_val) __attribute__ ((deprecated(use d_getenv_uint));

Copy link
Contributor Author

@knard38 knard38 Jan 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

  • Use compiler intrinsic macro.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • Use compiler intrinsic macro.

Fixed with commit e95531c

@kanard38
Merge remote-tracking branch 'origin/master' into ckochhof/hotfix/mas…
0703b18 
…er/daos-14981

Required-githooks: true
@kanard38
DAOS-14981 gurt: restore d_getenv_int undefined symbol
e95531c 
Integrate reviewers comments:
- Use compiler intrinsic macro

Features: mpiio dfuse
Required-githooks: true
Signed-off-by: Cedric Koch-Hofer <[email protected]>
@kanard38
DAOS-14981 gurt: restore d_getenv_int undefined symbol
8017cbe 
Fix clang-format coding style.

Features: mpiio dfuse
Required-githooks: true
Signed-off-by: Cedric Koch-Hofer <[email protected]>
@kanard38
DAOS-14981 gurt: restore d_getenv_int undefined symbol
981d4a1 
Fix clang-format coding style.

Features: mpiio dfuse
Required-githooks: true
Signed-off-by: Cedric Koch-Hofer <[email protected]>
@knard38 knard38 requested a review from ashleypittman January 17, 2024 13:57
@daosbuild1
Copy link
Collaborator

Test stage NLT on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-13622/2/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage Functional Hardware Medium Verbs Provider completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-13622/4/execution/node/1389/log

@daosbuild1
Copy link
Collaborator

Test stage Functional Hardware Medium completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-13622/4/testReport/

@knard38 knard38 requested a review from jolivier23 January 19, 2024 08:31
@knard38 knard38 requested a review from a team January 19, 2024 08:56
@knard38
Copy link
Contributor Author

knard38 commented Jan 19, 2024 •
edited
Loading

Added forced landing label as CI failures are known issues:

@knard38 knard38 added the forced-landing The PR has known failures or has intentionally reduced testing, but should still be landed. label Jan 19, 2024
@gnailzenh gnailzenh merged commit 1ed8f1d into master Jan 19, 2024
@gnailzenh gnailzenh deleted the ckochhof/hotfix/maser/daos-14981 branch January 19, 2024 13:06
jolivier23 pushed a commit that referenced this pull request Feb 28, 2024
@knard38 @jolivier23
DAOS-14981 gurt: restore d_getenv_int undefined symbol (#13622)
b4f9f8d 
* DAOS-14981 gurt: restore d_getenv_int undefined symbol

Restore missing plain function d_getenv_int() to fix missing symbol with
libdaos.

Signed-off-by: Cedric Koch-Hofer <[email protected]>
jolivier23 pushed a commit that referenced this pull request Mar 12, 2024
@knard38 @jolivier23
DAOS-14981 gurt: restore d_getenv_int undefined symbol (#13622)
43b921c 
* DAOS-14981 gurt: restore d_getenv_int undefined symbol

Restore missing plain function d_getenv_int() to fix missing symbol with
libdaos.

Required-githooks: true

Change-Id: I86d5c2f5d4d8bbd3c4ab3fdef70ffc5b41ce0921
Signed-off-by: Cedric Koch-Hofer <[email protected]>
jolivier23 pushed a commit that referenced this pull request Apr 10, 2024
@knard38 @jolivier23
DAOS-14981 gurt: restore d_getenv_int undefined symbol (#13622)
c268250 
* DAOS-14981 gurt: restore d_getenv_int undefined symbol

Restore missing plain function d_getenv_int() to fix missing symbol with
libdaos.

Required-githooks: true

Change-Id: I86d5c2f5d4d8bbd3c4ab3fdef70ffc5b41ce0921
Signed-off-by: Cedric Koch-Hofer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashleypittman ashleypittman ashleypittman approved these changes

@gnailzenh gnailzenh gnailzenh approved these changes

@frostedcmos frostedcmos frostedcmos approved these changes

@wangshilong wangshilong Awaiting requested review from wangshilong

@jolivier23 jolivier23 Awaiting requested review from jolivier23

Assignees

@knard38 knard38

Labels
forced-landing The PR has known failures or has intentionally reduced testing, but should still be landed.
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@knard38 @daosbuild1 @ashleypittman @gnailzenh @frostedcmos @kanard38