New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/link: making sense of HeadType #22270

Open
crawshaw opened this Issue Oct 14, 2017 · 3 comments

Comments

Projects
None yet
4 participants
@crawshaw
Contributor

crawshaw commented Oct 14, 2017

The linker makes extensive use of HeadType. It does not make much sense to me.

When used as a value, it is very close to meaning GOOS. One exception is both "android" and "linux" are represented as "Hlinux", but besides that they are equivalent.

When used as a flag to the linker, -H, it is the value of GOOS, plus the potential value "windowsgui", which is treated as "windows" except it sets a global linker variable windowsgui to true. The linker is also receiving the GOOS environment variable used by the compiler (and all other tools) to make their OS decision.

Proposal:

  • Leave the -H flag alone, as other build systems may depend on it.
  • Check when the linker starts that GOOS and -H match.
  • Convert any OS-specific conditions in the linker to using GOOS.
  • Convert the HeadType enumeration into:
    const (
          Unknown HeadType = iota
          ELF
          MachO
          Plan9
          PE
          PEGUI
    )
    
  • Replace the IsELF global variable with ctxt.HeadType == objabi.ELF instead.
  • Replace windowsgui with ctxt.HeadType == objabi.PEGUI.

Thoughts?

cc @ianlancetaylor

@rsc

This comment has been minimized.

Contributor

rsc commented Oct 14, 2017

SGTM.

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Oct 15, 2017

PE
PEGUI

I think you want PE only. PEGUI is not much different from PE. There are 2 places in cmd/link/internal/ld where that difference is important. On the other hand there are many places where Hwindows is used, so you would end up with code like (Headtype == PE || Headtype == PEGUI) instead of Headtype == PE everywhere. And if you forget the || Headtype == PEGUI part here or there, you won't get a broken test, because we don't have many tests that check PEGUI.

And PEGUI will be exported, unlike current windowsgui variable.

We already had PE and PEGUI split before, so you would be effectively revert CL 38761.

Alex

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Oct 15, 2017

As far as I know the notion of "head type" precedes the notion of GOOS.

Like Alex, I don't think we need PEGUI. The variable windowsgui (which should of course be in the Link struct) seems like the right approach to me. It's just a small variant of Windows, not worth calling out in such a visible way.

There's really no reason to call your new suggested enum HeadType but I suppose we can do that to puzzle future readers.

@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Mar 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment