[PATCH] D142660: [AIX] supporting -X options for llvm-ranlib in AIX OS

[Digger Lin via Phabricator via llvm-commits]

DiggerLin marked 3 inline comments as done.
DiggerLin added inline comments.


================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:1463
 
+  if (object::Archive::getDefaultKindForHost() == object::Archive::K_AIXBIG) {
+    // If not specify -X option, get BitMode from enviorment variable
----------------
jhenderson wrote:
> DiggerLin wrote:
> > jhenderson wrote:
> > > Is there a particular reason that this is after the command-line option parsing for llvm-ranlib, but before it in llvm-ar? If this were before the option parsing, you wouldn't need the `HasAixXOption` variable.
> > in AIX OS  `ranlib` has behavior as
> > 
> > 
> > ```
> > -bash-5.0$ env OBJECT_MODE=31 ranlib tmpk.a
> > 0654-603 The OBJECT_MODE environment variable has an invalid setting.
> >         OBJECT_MODE must be 32, 64, or 32_64.
> > -bash-5.0$ env OBJECT_MODE=31 ranlib -X32  tmpk.a
> > -bash-5.0$
> > ```  
> > 
> > Given invalid env OBJECT_MODE , if there is no -X option in the ranlib command, it will output as 
> > 
> > ```
> > 0654-603 The OBJECT_MODE environment variable has an invalid setting.
> >         OBJECT_MODE must be 32, 64, or 32_64.
> > ```
> > 
> > Given invalid env OBJECT_MODE , and there is -X option in the ranlib command,  it do not care about the invalid env OBJECT_MODE,  So I has to parse the -X option before the getBitMode(getenv("OBJECT_MODE"))
> > 
> So with AIX ar, does an invalid OBJECT_MODE environment variable get reported if the -X option is specified?
> 
> In my opinion, I think it is very unlikely there will be any real users out there with an invalid OBJECT_MODE environment variable, because other tools will reject it, even if ranlib doesn't. Even if there are, they should be able to easily fix their variable, if they start getting an error message after switching to llvm-ranlib. I'm therefore thinking that there isn't a need to mirror this logic in llvm-ranlib, if it would make the code simpler (which it would).
> So with AIX ar, does an invalid OBJECT_MODE environment variable get reported if the -X option is specified?

it do no report invalid OBJECT_MODE environment if the -X option is specified.


```
bash-5.0$ export OBJECT_MODE=31
bash-5.0$ ar  q tmpp.acheck.o
ar: 0707-133 The OBJECT_MODE environment variable has an invalid setting.
        OBJECT_MODE must be 32, 64, 32_64, d64, or any.
bash-5.0$ ar -X32 q tmpp.acheck.o
bash-5.0$
```



> In my opinion, I think it is very unlikely there will be any real users out there with an invalid OBJECT_MODE environment variable, because other tools will reject it, even if ranlib doesn't. Even if there are, they should be able to easily fix their variable, if they start getting an error message after switching to llvm-ranlib. I'm therefore thinking that there isn't a need to mirror this logic in llvm-ranlib, if it would make the code simpler (which it would).

since -X option will overwrite the env variable OBJECT_MODE, 
in the source code , we should check the -X option of the command line first, if there is no  -X option ,  we will check the env variable OBJECT_MODE. the logic of code is correct in the patch.  we should not always  getenv("OBJECT_MODE")  before checking the -X option.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142660/new/

https://reviews.llvm.org/D142660