[SERVER-48427] Fix useless compiler tools checks Created: 26/May/20  Updated: 27/Oct/23  Resolved: 13/Dec/21

Status: Closed
Project: Core Server
Component/s: Build
Affects Version/s: 4.5 Desired
Fix Version/s: None

Type: Improvement Priority: Major - P3
Reporter: Ryan Egesdahl (Inactive) Assignee: [DO NOT ASSIGN] Backlog - Server Development Platform Team (SDP) (Inactive)
Resolution: Works as Designed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Assigned Teams:
Server Development Platform
Participants:

 Description   

In both etc/scons/mongodbtoolchain_v3_gcc.vars and etc/scons/mongodbtoolchain_v3_clang.vars, we test that the toolchain is complete using the following:

AR = subprocess.check_output([CXX, '-print-prog-name=ar']).decode('utf-8').strip()
AS = subprocess.check_output([CXX, '-print-prog-name=as']).decode('utf-8').strip()
OBJCOPY = subprocess.check_output([CXX, '-print-prog-name=objcopy']).decode('utf-8').strip()

Ostensibly, this check makes sure the compiler toolchain is valid. However, there are two problems with this approach:

  1. We already do it when we test compile simple programs, so it's not really needed.
  2. It's not actually a valid test because:
    1. We're using piped builds and the toolchains determine what to execute at build time.
    2. We never set these variables in the environment to ensure what we tested is what we actually use during build.
    3. It's possible to override the tool that gets used regardless of what we test.

As they exist now, these tests are confusing because they declare something that isn't actually true. In especially the case of clang, it will use llvm-as on systems that support it, and we only ever test as. We should either remove the tests entirely or, if we still need them for some reason, update them to be accurate with respect to the actual build that will occur.



 Comments   
Comment by April Schoffer [ 13/Dec/21 ]

Discussed during planning - this is no longer relevant.

Comment by Ryan Egesdahl (Inactive) [ 27/May/20 ]

I didn't realize that these variables were being used elsewhere, but that does make sense, thanks. I think this works just fine for gcc based on what you're saying, but I don't think it works for llvm because we can end up in a situation where we're using two different linkers or assemblers in the same build. Is there a way we can pass tool names down to this script so we're just asking the compiler to resolve the tool path for us? I am not sure what a good practice here would be.

Comment by Andrew Morrow (Inactive) [ 27/May/20 ]

These aren't really validity checks though. We are asking the toolchain to tell us what it would use if it wanted to invoke the assembler or archiver, so that we can tell SCons where those tools are. There are (a few) situations where we manually invoke the assembler, so we need to know where the compiler's version of `as` is, and there are lots of circumstances where we directly invoke the archiver. Ditto objcopy. I agree that setting these doesn't change what the compiler will use. Note though that this file also sets the toolchain bin directory into the PATH. This is more of a safety net than a hard requirement: the toolchain should already know the sysroot by which to find its own builds of the linker and assembler (should it need one).

Generated at Thu Feb 08 05:17:05 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.