libpcp: fix PCP_ORIGIN_TIME environment variable name in pmGetOptions

[sfeifer]

__pmStartOptions() read getenv("ORIGIN_TIME") instead of getenv("PCP_ORIGIN_TIME"), so the origin time exported by the pcp(1) front-end was never picked up by subtools. Correct the name in both libpcp and libpcp3.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Advanced

Run ID: 14311c4a-923c-4824-92ce-f5374df85c3c

📥 Commits

Reviewing files that changed from the base of the PR and between aeb4255 and 78c4b46.

📒 Files selected for processing (2)

• src/libpcp/src/getopt.c
• src/libpcp3/src/getopt.c

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

📝 Summary

Summary by CodeRabbit

• Configuration
  • The environment variable for setting the origin timestamp is now PCP_ORIGIN_TIME.
  • ORIGIN_TIME is no longer used for this configuration.

Walkthrough

Both option parser implementations now use PCP_ORIGIN_TIME instead of ORIGIN_TIME for origin-time configuration and debug output.

Changes

Origin-time configuration

Layer / File(s)	Summary
Update origin-time lookup src/libpcp/src/getopt.c, src/libpcp3/src/getopt.c	__pmStartOptions reads PCP_ORIGIN_TIME, passes its value to __pmSetOrigin, and prints the new variable name in configuration traces.

Suggested reviewers: kmcdonell

Priority: ⬇️ Low

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 78c4b

This change aligns origin-time configuration in both libraries and presents no identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the correction to the PCP_ORIGIN_TIME environment variable in pmGetOptions. It matches the main changes in both libpcp and libpcp3.
Description check	✅ Passed	The description accurately explains the incorrect environment variable name, the affected function, the impact on subtools, and the correction in both libraries.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks the clock at dawn
The origin name is neatly drawn
PCP variables guide the way
Two parsers hop in sync today
Debug traces now say what they mean}

_{Comment @coderabbitai help to get the list of available commands.}

[natoscott]

hmm, this is odd - I wonder why we have ORIGIN_TIME in the first place? the corresponding command line option is --origin and not --origintime (unlike --starttime and --endtime).

Can we just remove this oddball ORIGIN_TIME case entirely and use PCP_ORIGIN instead?

[sfeifer]

After going through the code yesterday, I determined ORIGIN_TIME does not exist (I assume it was a typo). PCP_ORIGIN_TIME comes from src/pcp/pcp.sh and PCP_ORIGIN comes from src/pcp/summary/pcp-summary.sh. I think PCP_ORIGIN_TIME makes the most sense here and maybe we could even replace PCP_ORIGIN with PCP_ORIGIN_TIME in other places so we only use one of these, eliminating some confusion. @kmcdonell Any thoughts on this?

Also, it looks like Ken already fixed the typo causing the QA failures in the changes from last night, #2717, so we are safe to close this MR.

[kmcdonell]

@sfeifer and @natoscott ORIGIN_TIME is was in the old code and I has assumed it was for some backwards compatibility reason.
But I botched the getopts.c change when I converted the "extern environ or _environ" code to use getenv() and locking ... it was a cut-n-paste error. Although I see one diagnostic (in the two getopt.c files) still has ORIGIN_TIME, and I have a commit for that pending.
But if the only use of ORIGIN_TIME in our code base is src/pcp/summary/pcp-summary.sh where mapping -O to ORIGIN_TIME is just wrong and ORIGIN_TIME can simply go away ... since I started this whole saga, let me fix that as well and I think you can close this PR and I'll push those changes via another PR.
Sorry for the distraction.

[kmcdonell]

@sfeifer and @natoscott PCP_ORIGIN exists in more places (other src/pcp scripts, qa and man pages).
On reflection I think the right thing to do is to remove uses of it from the code base, keep the getopt.c support for backwards compatibility and mark it as "deprecated, may be removed in a future release" in the man page.
Thoughts?

[natoscott]

I think we need to keep PCP_ORIGIN (e.g. for pcp(1) frontend to pass -O through to backend tools, like all the other options) but remove PCP_ORIGIN_TIME - is that what we're thinking here?

[kmcdonell]

@natoscott I think we need both for backwards compatibility (at least for the moment).
My current (un-pushed) commit does the following:

1. keep support in both libpcp's for both variable names
2. use PCP_ORIGIN_TIME (more consistent with other PCP__TIME variables) everywhere in the code base, including qa and the pcp- frontends
3. mark PCP_ORIGIN as deprecated in the man page.

[natoscott]

Hmm, I think I prefer keeping these matching with the command line option naming. PCP_ORIGIN_TIME is a new thing that seems out of place for that reason. PCP_ORIGIN matches --origin exactly, so seems like the right thing to me (and should not be deprecated unless we plan to also deprecate that command line option naming and go with --origintime ... which doesn't have a great ring to it). The _TIME suffixed names match command line options with "time" at the end of the option name ... so they too seem tight to me as-is.

[kmcdonell]

@natoscott PCP_ORIGIN_TIME is not new, it was there in PCP 3.8 and maybe before. And PCP_ORIGIN was there then too.
PCP_ORIGIN_TIME or --origin is just like PCP_START_TIME or --start and PCP_FINISH_TIME or --finish and PCP_ALIGN_TIME or --align ... PCP_ORIGIN is the odd one out.
But I agree no need to deprecate (so I'll cull the man page change). And we're back to where we started before I hacked getopts.c.