[8.0.0 Release] One week to the branch

Hello everyone,

This is just a quick reminder that the upcoming release branch is
scheduled for creation one week from today, on Wednesday 16 January
2019.

The full schedule is available under "Upcoming Releases" in the right
column at https://llvm.org.

Please try to avoid committing disruptive changes just before or after
the branch.

As the branch is created, the trunk version will become 9.0.0.

Thanks,
Hans

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.

1 - Rip out the COFF support in the branch and let it error out as it did before.

2 - Add a custom patch for the branch, which makes the tool error out if an option that is known to be unimplemented/incomplete is specified, keeping the functionality that is known to be complete.

3 - Merge the llvm-objcopy/COFF patches to the release branch as things are completed in trunk during the rc phase, hopefully reaching a more complete state within that timeframe. As this is a new feature, it shouldn't have any stability impact on the rest (at least as long as we don't end up needing to patch other libraries in the process).

What do you think?

// Martin

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

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

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

Thanks! Yeah, it's a bit of a long list, and since it doesn't seem to
make a big difference whether this is in 8.0 or not, I think we should
skip it.

- Hans

Ok, fair enough. Thanks for your time in any case!

// Martin