llvm dev - Jan 2019 - [cfe-dev] [8.0.0 Release] One week to the branch

On Tue, 15 Jan 2019, Hans Wennborg wrote:
> On Sat, Jan 12, 2019 at 12:49 AM Jordan Rupprecht <rupprecht at
google.com> wrote:
>>
>>
>>
>> On Fri, Jan 11, 2019 at 6:26 AM Martin Storsjö <martin at
martin.st> wrote:
>>>
>>> Hi,
>>>
>>> The COFF support in llvm-objcopy is in a pretty half-finished state
at the
>>> moment. I had hoped to have it mostly usable for the common
scenarios by
>>> the time of the branch (the initial patch was sent at the end of
>>> November), but it's still lacking stripping of sections (while
stripping
>>> of symbols is pretty much done) and a few other minor features I
have
>>> waiting to be polished up according to review comments.
>>>
>>> When used right now, it won't error out or warn about not doing
what
>>> actually was requested, but just copy the object/executable and do
some
>>> symbol removals if requested.
>>>
>>> With the branch coming real soon, what's the preferred course
of action?
>>>
>>> 0 - Do nothing, release this as is. As llvm-objcopy hasn't
supported COFF
>>> before, nobody will try it and run into the issue.
>>
>> Option 0 seems fine to me
>
> I think this is fine too. If you want you could add a note somewhere
> that the COFF support is experimental and incomplete.
FWIW, I've got the main COFF functionality that I had planned on doing 
committed in trunk by now. So at least for my own usecases, it's fully 
functional by now. (And for unsupported options, it clearly errors out.)

If there's an interest in this, it should be easy to backport to the 
release branch (with no regression risk, as I believe none of the patches 
since the branch touch anything outside of the COFF directory), but I 
don't have a direct need myself to have it in the release.

// Martin

On Thu, Jan 24, 2019 at 3:51 AM Martin Storsjö <martin at martin.st>
wrote:
>
> On Tue, 15 Jan 2019, Hans Wennborg wrote:
>
> > On Sat, Jan 12, 2019 at 12:49 AM Jordan Rupprecht <rupprecht at
google.com> wrote:
> >>
> >>
> >>
> >> On Fri, Jan 11, 2019 at 6:26 AM Martin Storsjö <martin at
martin.st> wrote:
> >>>
> >>> Hi,
> >>>
> >>> The COFF support in llvm-objcopy is in a pretty half-finished
state at the
> >>> moment. I had hoped to have it mostly usable for the common
scenarios by
> >>> the time of the branch (the initial patch was sent at the end
of
> >>> November), but it's still lacking stripping of sections
(while stripping
> >>> of symbols is pretty much done) and a few other minor features
I have
> >>> waiting to be polished up according to review comments.
> >>>
> >>> When used right now, it won't error out or warn about not
doing what
> >>> actually was requested, but just copy the object/executable
and do some
> >>> symbol removals if requested.
> >>>
> >>> With the branch coming real soon, what's the preferred
course of action?
> >>>
> >>> 0 - Do nothing, release this as is. As llvm-objcopy hasn't
supported COFF
> >>> before, nobody will try it and run into the issue.
> >>
> >> Option 0 seems fine to me
> >
> > I think this is fine too. If you want you could add a note somewhere
> > that the COFF support is experimental and incomplete.
>
> FWIW, I've got the main COFF functionality that I had planned on doing
> committed in trunk by now. So at least for my own usecases, it's fully
> functional by now. (And for unsupported options, it clearly errors out.)
>
> If there's an interest in this, it should be easy to backport to the
> release branch (with no regression risk, as I believe none of the patches
> since the branch touch anything outside of the COFF directory), but I
> don't have a direct need myself to have it in the release.
Since it's low-risk and finishing up functionality, if it's just a
small amount of patches we might as well merge it over. Do you have a
list of what commits are involved?

On Thu, 24 Jan 2019, Hans Wennborg wrote:
> On Thu, Jan 24, 2019 at 3:51 AM Martin Storsjö <martin at martin.st>
wrote:
>>
>> FWIW, I've got the main COFF functionality that I had planned on
doing
>> committed in trunk by now. So at least for my own usecases, it's
fully
>> functional by now. (And for unsupported options, it clearly errors
out.)
>>
>> If there's an interest in this, it should be easy to backport to
the
>> release branch (with no regression risk, as I believe none of the
patches
>> since the branch touch anything outside of the COFF directory), but I
>> don't have a direct need myself to have it in the release.
>
> Since it's low-risk and finishing up functionality, if it's just a
> small amount of patches we might as well merge it over. Do you have a
> list of what commits are involved?
In SVN revisions, it's the following:

351657
351658
351659
351660
351661
351662
351663
351799
351800
351801
351811
351931
351934
351946
351947
351948

In a git mirror, it's trivial to find these commits with the following 
command:

git log $(git merge-base origin/release_80 master)..master
tools/llvm-objcopy/COFF test/tools/llvm-objcopy/COFF

Among these commits, there's one cycle of "Implement
--add-gnu-debuglink",
"Revert 'Implement --add-gnu-debuglink'", "Reapply:
Implement
--add-gnu-debuglink", "Remove testcase debugging lines. NFC.". If
you
prefer, you can pick all of them to keep it similar to trunk, or you can 
just pick the first one of them, since the latter three end up a no-op. 
(The fix for the issue that caused the revert is in r351934.)

Among the commits listed by git, there's one generic llvm-objcopy 
refactoring that I didn't list above ("Return Error from 
Buffer::allocate(), [ELF]Writer::finalize(), and [ELF]Writer::commit()"), 
and the license header change that shouldn't be backported. After 
backporting the commits to the branch, the diff to the trunk version of 
the tools/llvm-objcopy/COFF directory is only these two commits.

So, not exactly trivial and minimal, but still pretty well isolated.

// Martin