PMM-13540 Fix pmm client v3 build for nomad.

[talhabinrizwan]

PMM-13540

Link to the Feature Build: SUBMODULES-3766

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 43.53%. Comparing base (6bfa326) to head (4127d38).
Report is 1 commits behind head on v3.

Additional details and impacted files

@@            Coverage Diff             @@
##               v3    #3298      +/-   ##
==========================================
- Coverage   43.54%   43.53%   -0.01%     
==========================================
  Files         366      366              
  Lines       44162    44162              
==========================================
- Hits        19232    19228       -4     
- Misses      23249    23252       +3     
- Partials     1681     1682       +1     

Flag	Coverage Δ
admin	11.47% <ø> (-0.05%)	⬇️
agent	51.96% <ø> (ø)
managed	45.25% <ø> (-0.01%)	⬇️
vmproxy	68.57% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ademidoff]

This declaration makes ARCH a global variable. Can we move the whole block to extract_source_tarball and declare ARCH as a local variable?

[talhabinrizwan]

I opted to keep ARCH as a global variable since we're currently building the client for multi-architecture, and having it set globally may streamline future adjustments. However, I'm open to making it local to another function if necessary.

Could you clarify why you suggest moving ARCH specifically to the extract_source_tarball function? At present, ARCH is only used in the gobuild_component function.

[ademidoff]

No, it's used elsewhere, and it's also global there. I suggested this since global variables are normally considered evil - you won't know in case one overrides the other. Moreover, such collisions are difficult to troubleshoot.

As a general rule, defaulting to use local variables is a good programming practice no matter what the language is, including bash scripting.

[talhabinrizwan]

I understand that global variables can lead to conflicts and troubleshooting issues due to potential overrides, which is why they’re generally avoided in programming. However, there are cases in scripting, especially in bash where global variables simplify tasks by making key parameters available across functions. In this case, setting ARCH globally could streamline changes if we expand to multi-architecture builds in the future. So while local variables are often preferable, some situations benefit from a well-managed global variable. That said, I’m okay with moving it into the function, would you like me to place it in the gobuild_component function?

[ademidoff]

Using global vars across multiple scripts requires a good structure. Usually, globals are not declared everywhere, otherwise they become difficult to understand and maintain.

Making ARCH a global variable would necessitate declaring it in vars script, which is the proper place for globals, and which should be the only place for globals. Also, it would be necessary to refactor the use of this variable across other scripts.

Apart from that, we are undertaking a heavy refactoring of the build scripts, which will hopefully make it possible to build an arm/aarch flavor of PMM as well. I suggest we address it there.

That said, I suggest we make it local for now.