Skip to content
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

envoy: remove linux build #87142

Closed

Conversation

codefromthecrypt
Copy link
Contributor

@codefromthecrypt codefromthecrypt commented Oct 12, 2021

I don't expect this change to be universally popular. However, I think
it is the most pragmatic way to isolate the impact of envoy on
maintainers and end users. Specifically, this removes the linux build
and in doing so removes fragile and complex configuration. It also will
reduce the time to land new versions on OS/x.

I believe this is justified beyond the technical dept recovered. Envoy
is not often installed on linux with homebrew. To the degree it can be
used, it can't be used on arm64 which is a supported Envoy platform on
Linux. The few using linuxbrew have other options including the official
docker images, func-e, and in the future, formal system packages and
tarballs.

Once we remove this, maintaining homebrew envoy's primary audience, OS/x
is easier and faster. We can focus energy on what's not available
elsewhere: a macOS build of Envoy. We can focus on the larger audience
which extends larger still when Envoy completes arm64+osx (likely in
version 1.21). Maintainers do not have to run very long linux builds
anymore, and CI resources can be conserved for projects more often used.

More details on the discussion are here, but I'm raising this PR as a
call to action. If the homebrew leads decide it is better to keep trying
to build on Linux, the main impact is continued longer and more complex
work to release new versions to end users. That's not the worst outcome,
but I believe they deserve better, hence this issue.

See Homebrew/discussions#2271

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

I don't expect this change to be universally popular. However, I think
it is the most pragmatic way to isolate the impact of envoy on
maintainers and end users. Specifically, this removes the linux build
and in doing so removes fragile and complex configuration. It also will
reduce the time to land new versions on OS/x.

I believe this is justified beyond the technical dept recovered. Envoy
is not often installed on linux with homebrew. To the degree it can be
used, it can't be used on arm64 which is a supported Envoy platform on
Linux. The few using linuxbrew have other options including the official
docker images, func-e, and in the future, formal system packages and
tarballs.

Once we remove this, maintaining homebrew envoy's primary audience, OS/x
is easier and faster. We can focus energy on what's not available
elsewhere: a macOS build of Envoy. We can focus on the larger audience
which extends larger still when Envoy completes arm64+osx (likely in
version 1.21). Maintainers do not have to run very long linux builds
anymore, and CI resources can be conserved for projects more often used.

More details on the discussion are here, but I'm raising this PR as a
call to action. If the homebrew leads decide it is better to keep trying
to build on Linux, the main impact is continued longer and more complex
work to release new versions to end users. That's not the worst outcome,
but I believe they deserve better, hence this issue.

See Homebrew/discussions#2271

Signed-off-by: Adrian Cole <adrian@tetrate.io>
@BrewTestBot BrewTestBot added no ARM bottle Formula has no ARM bottle no Linux bottle Formula has no Linux bottle labels Oct 12, 2021
@SMillerDev
Copy link
Member

Despite my objections to the concept, we shouldn't remove all Linux compatible code, we can just add a depends_on :macos and that will ensure nobody can run envoy on Linux.

In my mind only supporting 2/5 homebrew platforms qualifies a formula for deletion though.

@codefromthecrypt
Copy link
Contributor Author

codefromthecrypt commented Oct 12, 2021

@SMillerDev thanks for your feedback, and I have some follow up

Despite my objections to the concept, we shouldn't remove all Linux compatible code, we can just add a depends_on :macos and that will ensure nobody can run envoy on Linux.

Wouldn't this keep the maintenance and tech debt as well the special runner? Maybe I'm misunderstanding, but this would prevent people from using it, but also burden the maintainers like me equally on every release as if the build fails it fails right? What am I missing?

In my mind only supporting 2/5 homebrew platforms qualifies a formula for deletion though.

I would agree, though I think it is more like 4/5 by next release.

10.14 seems to only fail checking the homepage. I think that's a false failure unless someone knows another reason why it shouldn't build. I have no idea why that fails, but I am just guessing cert related?

probably skipping record,. but the arm64 thing is pending one pull request that was done in July, but agreed not useful until merged. I think there's a good chance envoy will actually merge that, as it has continual updates. In other words, it isn't abandoned.

So, my thinking is that if somehow we can get the lint check on envoy's 1.14 to pass, that's 3/5 and very likely by next release arm64 would be working so that's 4/5. At that point probably envoy's formula is less at the bottom of the pack and more in the middle top, right?

@SMillerDev
Copy link
Member

but also burden the maintainers like me equally on every release as if the build fails it fails right? What am I missing?

It wouldn't try and build because it depends on macOS in that case

@codefromthecrypt
Copy link
Contributor Author

It wouldn't try and build because it depends on macOS in that case

ah I understand now. The if statements and GCC overrides for linux will have a high drift risk, as I'm not sure how many will build from source something that takes hours to complete and is available elsewhere.

That said, I'm personally ok adjusting the logic so that the least amount of if's are used and so least drift. In other words the main goal I have is to make it easier and less time consuming to get releases out. If leaving the linux stuff in here does that, all good!

@codefromthecrypt
Copy link
Contributor Author

working on this still. can't say I think it is worthwhile as I really think it is definitely not. See #82077 (comment)

@codefromthecrypt
Copy link
Contributor Author

I'll try to figure out if linux really needs the python3 overriding, or if it was a mistake, or if it is only needed if you are using homebrew/ubuntu16.04, which after this PR won't be the case.

@codefromthecrypt
Copy link
Contributor Author

codefromthecrypt commented Oct 13, 2021

So, the thing with homebrew/ubuntu16.04 is not that python3 can't be found, it is that the floor version of it isn't 3.5 and that's what's on the homebrew/ubuntu16.04 image. I'll see if this problem happens if you are running normal ubuntu:16.05, though iirc that doesn't install python3 by default..

==> /home/linuxbrew/.linuxbrew/opt/bazelisk/bin/bazelisk build --compilation_mode=opt --curses=no --show_task_finish --verbose_failures --action_env
Last 15 lines from /home/linuxbrew/.cache/Homebrew/Logs/envoy/01.bazelisk:
    exec(code, run_globals)
  File "/tmp/envoy-20211013-11014-1hkv4c2/.brew_home/_bazel/2931c46125cf9af168cbcf060ba9d0f4/external/rules_python/python/pip_install/extract_wheels/__main__.py", line 5, in <module>
    main()
  File "/tmp/envoy-20211013-11014-1hkv4c2/.brew_home/_bazel/2931c46125cf9af168cbcf060ba9d0f4/external/rules_python/python/pip_install/extract_wheels/__init__.py", line 87, in main
    subprocess.run(pip_args, check=True)
  File "/usr/lib/python3.5/subprocess.py", line 708, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '['/usr/bin/python3', '-m', 'pip', 'wheel', '-r', '/tmp/envoy-20211013-11014-1hkv4c2/.brew_home/_bazel/2931c46125cf9af168cbcf060ba9d0f4/external/envoy/source/extensions/filters/network/kafka/requirements.txt', '--require-hashes']' returned non-zero exit status 1
)
ERROR: Analysis of target '//source/exe:envoy-static' failed; build aborted: Analysis failed
INFO: Elapsed time: 115.773s
INFO: 0 processes.

@danielnachun
Copy link
Member

We don't use system Python on Linux because we can't control what version the user has (if they even have one). Using a specific version of GCC is also not an issue - we have to do this all the time. We have several maintainers (including myself) with extensive knowledge of GCC and Python, and we have far more complex Linux-specific code in other formulae.

If you are hitting Linux build problems in the future please tag the Linux maintainers with @Homebrew/linux (or ask someone to do it if you don't have permission - I can't recall if non-maintainers can use that tag).

@codefromthecrypt
Copy link
Contributor Author

These things are an issue as things break routinely and as discussed in this issue and many others they cause delays for questionable merit. Even building from source is mysterious. For example, try using brew install --build-from-source envoy on a fresh linux image, you get into a loop where it tells you to install gcc and regardless of whether you do that and if a versioned or not, it keeps saying the same thing.

there are problems with the design of this formula and how python is setup. it caused a break due to a maintenance sweep gone wrong. This is because it is using python only on linux. And many failures take hours to figure out because the build of envoy is so extremely slow.

I don't understand why there is such a resistance to rid this burden, but I'm getting that there is. Even if almost no one uses this linux part of the formula it should be defended. I can't spend every release a week on this stuff. I think the idea of the linux formula of envoy grew its own legs, it is more important a. than if anyone uses it and b. if it causes routine maintenance and build breaks.

I'll just close this, as it feels all pointless. we'll just wait for the next breaks to occur and plan a week delay every time like we have been.

@codefromthecrypt codefromthecrypt deleted the remove-envoy-linux branch October 13, 2021 03:00
@carlocab
Copy link
Member

For reference:

❯ curl --location --silent https://formulae.brew.sh/api/formula-linux/envoy.json | jq '.analytics'
{
  "install": {
    "30d": {
      "envoy": 4
    },
    "90d": {
      "envoy": 30
    },
    "365d": {
      "envoy": 88
    }
  },
  "install_on_request": {
    "30d": {
      "envoy": 4
    },
    "90d": {
      "envoy": 30
    },
    "365d": {
      "envoy": 88
    }
  },
  "build_error": {
    "30d": {
      "envoy": 0
    }
  }
}

On macOS:

{
  "install": {
    "30d": {
      "envoy": 477
    },
    "90d": {
      "envoy": 1739
    },
    "365d": {
      "envoy": 5725
    }
  },
  "install_on_request": {
    "30d": {
      "envoy": 477
    },
    "90d": {
      "envoy": 1742
    },
    "365d": {
      "envoy": 5729
    }
  },
  "build_error": {
    "30d": {
      "envoy": 0
    }
  }
}

@danielnachun
Copy link
Member

These things are an issue as things break routinely and as discussed in this issue and many others they cause delays for questionable merit. Even building from source is mysterious. For example, try using brew install --build-from-source envoy on a fresh linux image, you get into a loop where it tells you to install gcc and regardless of whether you do that and if a versioned or not, it keeps saying the same thing.

No end users need to build this from source on Linux - the bottle is :any_skip_relocation so all users can use the bottle, even if in a non-default prefix. We normally do local testing of builds in our CI Docker image with docker run -it homebrew/ubuntu16.04:master, which helps us make sure it will work in CI.

there are problems with the design of this formula and how python is setup. it caused a break due to a maintenance sweep gone wrong. This is because it is using python only on linux. And many failures take hours to figure out because the build of envoy is so extremely slow.

This is unfortunate, but we maintain many formulae with long build times on both macOS and Linux. That's not a reason not to support a particular platform.

I don't understand why there is such a resistance to rid this burden, but I'm getting that there is. Even if almost no one uses this linux part of the formula it should be defended. I can't spend every release a week on this stuff. I think the idea of the linux formula of envoy grew its own legs, it is more important a. than if anyone uses it and b. if it causes routine maintenance and build breaks.

I'll just close this, as it feels all pointless. we'll just wait for the next breaks to occur and plan a week delay every time like we have been.

A one week turnaround is quite good for getting a new release package for a large complex package like envoy. It took us several months to get Qt 6 packaged, and quite a few other Linux distros have not even packaged it yet. It doesn't look like Debian, Fedora or Arch have packaged envoy, so I think we're doing pretty well given our finite resources. Please keep in mind that maintaining formulae is a collective effort on the part of all Homebrew maintainers but we are all volunteers, so things can't always move as fast as some would like.

@codefromthecrypt
Copy link
Contributor Author

keep in mind when you talk about volunteers I am a volunteer and my lost weekends matter also. This is not purely about whether or not you personally are able to do something or if there are things that are worse. There's a collective debt and while you may think 4 installs in 30 days is worth my last week of labor, I don't.

@carlocab
Copy link
Member

carlocab commented Oct 13, 2021

Thanks for all the work you've put into maintaining the Envoy formula, @codefromthecrypt. I don't want you to feel like you're wasting your time.

If we face more build issues on Linux on the next version bump, even after giving the Linux maintainers a chance to have a look, I'm in favour of getting rid of the Linux build. I don't think this much time and effort (from anyone) is worth the 5 Linux users using the Envoy formula, nor is it worth significant delays to shipping updates to thousands of macOS users.

Qt is different because it at least has a few hundred (if not thousand) Linux users, so I don't think this is quite comparable.

@danielnachun
Copy link
Member

That's true - I think the biggest issue here was that it seemed that Linux maintainers were not made aware that this was the blocker. @codefromthecrypt please tag me or @Homebrew/linux if you hit build failures again. It's actually uncommon for Linux to be the more difficult platform to build on.

One thing that may help get this to work better on Linux is to use llvm as the Arch Linux AUR script does. Since this was last tried to there have been a lot of changes and improvements to llvm on Linux so that may smooth out the C++ issues. I'm also going to take a look at what's going on with the PATH in bazel because it shouldn't need to use any host OS binaries. Hopefully we can make this formula more stable on Linux so that it's not a burden in the future!

@github-actions github-actions bot added the outdated PR was locked due to age label Nov 14, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
no ARM bottle Formula has no ARM bottle no Linux bottle Formula has no Linux bottle outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants