Page MenuHomePhabricator

[docs][llvm-ar] Update llvm-ar command guide
ClosedPublic

Authored by gbreynoo on Oct 15 2019, 10:09 AM.

Details

Summary

The llvm-ar command guide has not been updated in some time, it is missing current functionality and contains information that is out of date.
This diff:

  • Updates the use of reStructuredText directives, as seen in other tools command guides.
  • Updates the command synopsis.
  • Updates the descriptions of the tool behaviour.
  • Updates the options section.
  • Adds details of MRI script functionality.
  • Removes the sections "Standards" and "File Format", they appear to be out of date and out of scope for a command guide. I can't find these sections in the other tool command guides.

Diff Detail

Event Timeline

gbreynoo created this revision.Oct 15 2019, 10:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 15 2019, 10:09 AM

Mostly nits, I have a couple larger suggestions that I'm ok skipping because better documentation is better than bad documentation :)

llvm/docs/CommandGuide/llvm-ar.rst
15

nit: missing space after period before "It"

28–29

I compared what options were provided on the GNU ar on my workstation, and found the following not implemented by llvm-ar:

[f] - truncate inserted filenames
[O] - display offsets of files
[V] - display version

(actually, we should note the first two, but just implement llvm-ar V... I might take a look at that now)

I *think* we do actually implement all of them, i.e. there are none that we "implement but ignore for compatibility", but it would be good to double check that -- if there are any like that, we should also add them here.

36–38

Is this a difference anymore? It sounds like this used to be a limitation that no longer holds.

39

Additionally llvm-ar is deterministic by default, vs GNU ar (and GNU binutils in general), which is only deterministic if it's been configured that way at build time (and I think the default is non-deterministic). So I think that's an important thing to call out. For example:

*Deterministic Archives*

By default, :program:`llvm-ar` always uses zero for timestamps and UIDs/GIDs to write archives in a deterministic mode.
This is equivalent to the :option:`D` modifier being enabled by default. If you wish to maintain compatibility with other
:program:`ar` implementations, you can pass the :option:`U` modifier to write actual timestamps and UIDs/GIDs.
324

This is a dead link for me - https://llvm.org/docs/CommandGuide/ar.html

rupprecht added inline comments.Oct 15 2019, 2:36 PM
llvm/docs/CommandGuide/llvm-ar.rst
28–29

See D69007 for [V]

ruiu added a comment.Oct 16 2019, 2:09 AM

Thank you for doing this!

llvm/docs/CommandGuide/llvm-ar.rst
36–38

I thought that too.

55

I see why you named this section "Long Options", but the section that would have been named "short options" is actually named "operations", and this section contains short options such as -h, so this section name seems a bit odd to me.

gbreynoo updated this revision to Diff 225236.Oct 16 2019, 8:48 AM

Fix nits, add missing options, remove old limitation and add information regarding deterministic archives.

gbreynoo marked 2 inline comments as done.Oct 16 2019, 8:57 AM
gbreynoo added inline comments.
llvm/docs/CommandGuide/llvm-ar.rst
55

I think the distinction between "Operation", "Modifier" and "Other" is helpful to the user but I'm not sure what to call this "Other" section. "Non key letter" feels long winded, and -h doesn't fit that either.

Maybe it should be called "Other" and put underneath the other two sections. M and h could be moved to the "Operation" section and --help would be considered a synonym. What do you think?

324

This was a carry over from the current guide. Should a link to the gnu-ar command guide be put here instead or would it be best to remove the section all together?

rupprecht accepted this revision.Oct 16 2019, 5:13 PM

Thanks! Please wait for Rui to take another look, but this looks good enough to me.

llvm/docs/CommandGuide/llvm-ar.rst
41

Since GNU ar seems to also accept but ignore this arg, is it worth mentioning at all? Or are there other archiver programs that do use it?

324

I think we can just remove it. All the other docs in the "GNU binutils replacements" sections only have a "See also" section if they refer to other llvm tools (e.g. llvm-objcopy refers to llvm-strip). We already advertise heavily in the intro that this is a suitable replacement for GNU ar.

This revision is now accepted and ready to land.Oct 16 2019, 5:13 PM
MaskRay added inline comments.Oct 16 2019, 6:47 PM
llvm/docs/CommandGuide/llvm-ar.rst
106

P can affect d.

Can T affect the d operation?

152

P affects r

209

L is an extension. Shall we mention the fact?

ruiu accepted this revision.Oct 16 2019, 8:21 PM

LGTM

llvm/docs/CommandGuide/llvm-ar.rst
55

I think "Other" is a better name.

gbreynoo updated this revision to Diff 225429.Oct 17 2019, 7:31 AM

Renamed "Long Options" to "Other" and moved it below the other options, removed bad link to gnu-ar and updated with changes from D69087.

gbreynoo marked 2 inline comments as done.Oct 17 2019, 7:49 AM
gbreynoo added inline comments.
llvm/docs/CommandGuide/llvm-ar.rst
41

I know this is mentioned in the help text, but as this effects Command Line I think it belongs here. I can't tell you which archiver used this option originally.

106

As a general modifier I haven't been explicit with which commands use P, this is consistent with the other general modifiers. Regarding T with D, current llvm-ar behaviour will convert thin to full archives if T is not included with a write command. I'm not sure if this is the behaviour we want.

gbreynoo updated this revision to Diff 225434.Oct 17 2019, 7:50 AM

Fixed two format issues.

MaskRay added inline comments.Oct 19 2019, 6:47 AM
llvm/docs/CommandGuide/llvm-ar.rst
9

Add O

MaskRay accepted this revision.Oct 19 2019, 6:59 AM
MaskRay added inline comments.
llvm/docs/CommandGuide/llvm-ar.rst
9

<archive> -> archive

A mandatory argument is not surrounded by <>. See objcopy and llvm-objcopy for an example.

This revision was automatically updated to reflect the committed changes.