nmeum / android-tools

Unoffical CMake-based build system for android command line utilities
Apache License 2.0
176 stars 51 forks source link

Include go vendor modules for boringssl #107

Closed munix9 closed 12 months ago

munix9 commented 1 year ago

Required for services that do not allow network access to the outside during compilation (e.g. open build service etc.). When compiling, at least an export GOFLAGS="-mod=vendor" must be done before.

See https://github.com/nmeum/android-tools/issues/47

nmeum commented 1 year ago

This only changes the CI but not the CMake files. So it does not affect a standard CMake build, right? I thought #47 was about the CMake build system (CC: @vanaf)?

munix9 commented 1 year ago

This only changes the CI but not the CMake files. So it does not affect a standard CMake build, right? I thought #47 was about the CMake build system (CC: @vanaf)?

That's right. It uses go mod vendor to ensure that the required go modules are downloaded and made available in the vendor/boringssl/vendor directory. Then when the appropriate android-tools-*.tar.xz package is built with make package_source, the modules are now included as well and can be considered in the build with export GOFLAGS="-mod=vendor".

Biswa96 commented 1 year ago

Q. Should those go modules be added in the same tarball? If those go modules are added in different tarball, it'll provide distributions a choice to use their own package/version.

munix9 commented 1 year ago

One possibility would be like gocryptfs does: provide an additional package which contains the described modules, e.g.: android-tools-*.tar.xz - as before, without the modules android-tools-*-deps.tar.xz - with the additional modules

The question is whether this is necessary to provide two different packages. If no export GOFLAGS="-mod=vendor" is set during the build process, the modules are downloaded by go. So if someone doesn't want to use the included modules (for whatever reason), they just omit the export GOFLAGS="-mod=vendor" - that's how I think it works.

See also https://go.dev/ref/mod#go-mod-vendor

munix9 commented 1 year ago

I have created an alternative with two separate packages.

diff: https://github.com/nmeum/android-tools/compare/master...munix9:android-tools:vendor3#diff artifacts: https://github.com/munix9/android-tools/actions/runs/4315918286

Let me know what you think about this.

JamiKettunen commented 1 year ago

I can confirm the android-tools-34.0.0-deps.tar.xz mentioned in above links allowed me to build for Chimera Linux without requiring internet, just setting env = {"GOFLAGS": "-mod=vendor"} was enough :)

I would like to add that excluding setting GOFLAGS in env caused it to still attempt the download, so I think having them in one tarball should be fine.

Meanwhile until this is sorted out I'll keep using the following workaround with the official source though:

def do_prepare(self):
    with self.pushd("vendor/boringssl"):
        self.do("go", "mod", "vendor", allow_network = True)
nmeum commented 1 year ago

I have created an alternative with two separate packages.

diff: master...munix9:android-tools:vendor3#diff artifacts: https://github.com/munix9/android-tools/actions/runs/4315918286

Let me know what you think about this.

So the idea here would be to have the CI generate the source tarball, include vendored Go dependencies in the tarball, and then use that tarball for the release? Sounds ok too me, can we also use the CI to automatically create a GitHub release when a new tag is pushed and use the created tarball for this release (if I understand your setup correctly then the tarball is only made available as a CI artifact presently)?

Alternatively: Could the go mod vendor step be integrated into the make package_source Makefile target? If we ship the release source tarball with vendored Go dependencies we should probably also use GOFLAGS with -mod=vendor by default I guess?

munix9 commented 1 year ago

I actually wanted to keep it simple, so I put it inside the .github/workflows/build.yml. I don't think it's possible to integrate it into CPACK_*.

What might be possible would be an additional cmake option, for example (untested):

CMakeLists.txt

option(ENABLE_GO_VENDOR "Packaging go vendor modules? (Default: off)" OFF)

if(ENABLE_GO_VENDOR)
  message(STATUS "Packaging go vendor modules")
  execute_process(
    COMMAND go mod vendor
    WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}/vendor/boringssl"
  )
endif()

# CPack configuration for creating source tarballs which already include

.github/workflows/build.yml

      - name: build & install
        run: |
          mkdir build && cd build
          cmake -DENABLE_GO_VENDOR=ON ..
          make package_source

...

      - name: build & install
        run: |
          export GOFLAGS="-mod=vendor"
          tar -xf android-tools-*.tar.xz
nmeum commented 1 year ago

I actually wanted to keep it simple, so I put it inside the .github/workflows/build.yml. I don't think it's possible to integrate it into CPACK_*.

Ok, that's fine. But is the idea to include vendored Go dependency in the release tarball (i.e. the one that can be downloaded from GitHub as well) or should they just be available as a CI artifact? Because the former is presently generated and uploaded manually by me using make package_source.

At Alpine we are usually also fine with downloading Go/Rust/… dependency from the internet as long as the version is fixed and the checksum is verified (ensuring that the build is reproducible). Not sure how other distributions handle it.

munix9 commented 1 year ago

Ok, that's fine. But is the idea to include vendored Go dependency in the release tarball (i.e. the one that can be downloaded from GitHub as well) or should they just be available as a CI artifact? Because the former is presently generated and uploaded manually by me using make package_source.

Yes, valid point, I had not considered the creation of a release. Main concern is to provide a complete archive as release with source code + vendor modules.

How about a script (similar to package-release-tarballs.bash), e.g.

#!/bin/bash

set -eu

cd "$(dirname "$0")"

_gitv=$(git describe --tags)

clean_build() {
        rm -rf build build-deps vendor/boringssl/vendor
}

package_source() {
        local mode="${1:-}"
        test -n "$mode" && {
                pushd vendor/boringssl
                go mod vendor
                popd
        }
        mkdir build${mode}
        pushd build${mode}
        cmake ..
        make package_source
        mv android-tools-${_gitv}.tar.xz ../android-tools-${_gitv}${mode}.tar.xz
        popd
}

#if git describe --dirty | grep dirty ; then
#       echo "Tree is dirty - I will not package this!"
#       exit 1
#fi

clean_build
package_source
package_source -deps
clean_build

The script creates two archives, one with source code only and another archive with source code AND vendor modules. I followed the example of https://github.com/rfjakob/gocryptfs/releases, which also offers two different packages.

Maybe you already have something similar for building a release.

At Alpine we are usually also fine with downloading Go/Rust/… dependency from the internet as long as the version is fixed and the checksum is verified (ensuring that the build is reproducible). Not sure how other distributions handle it.

With openSUSE, a network connection is not allowed during the build. The package maintainer has to make sure that additionally needed packages are either already available in the distribution or provide an appropriate vendor archive. With Fedora it is similar, I think.

nmeum commented 1 year ago

How about a script (similar to package-release-tarballs.bash), e.g.

While at it anyhow, my preferred solution would be to automate release tarball creation via the CI and as part of that also include the vendored Go dependencies. So every time a new tag/release is created, the CI would vendor the Go dependencies (analog to what you propose in this PR), create a new tarball, and upload that as an artifact for the tag/release. If the CI uses a script internally that would be fine but it would be cool if the tarball creation and upload for a release would be automated.

I think Go software on GitHub (like gocryptfs which you mentioned) often has similar rules in their CI setups to attach statically linked binaries (for different operating systems) to their GitHub releases. For example, the encryption tool age uses gh release upload in their CI workflow for this purpose: https://github.com/FiloSottile/age/blob/5d5c9c48d8d0c0707b529dc653e3289521ec5bb3/.github/workflows/build.yml#L83

The gocryptfs repository also does that but it seems that their static binaries are not automatically uploaded by the CI? Do you think that something similar to the age workflow would be viable here too? If so, would you be interested in updating your PR accordingly?

With openSUSE, a network connection is not allowed during the build. The package maintainer has to make sure that additionally needed packages are either already available in the distribution or provide an appropriate vendor archive. With Fedora it is similar, I think.

Oh! Good to know, in that case this is definitely something we should address.

munix9 commented 1 year ago

Sure, some kind of automation makes sense. I can take a look at it, but it may take some time. Until then I would suggest to publish the releases as usual.

nmeum commented 1 year ago

Sure, some kind of automation makes sense. I can take a look at it, but it may take some time. Until then I would suggest to publish the releases as usual.

Sure that's fine. I will do the platform-tools-34.0.1 release as usual then today.

JamiKettunen commented 12 months ago

Is there still something to do here after https://github.com/nmeum/android-tools/releases/tag/34.0.4 which contains the vendored boringssl sources now?

Biswa96 commented 12 months ago

...which contains the vendored boringssl sources now?

This is about the go modules in the boringssl project. And the 34.0.4 does not contain those.

JamiKettunen commented 12 months ago

Huh, I commented that as I no longer needed the setup I described previously (https://github.com/nmeum/android-tools/pull/107#issuecomment-1512932253) for my 34.0.4 packaging (https://github.com/chimera-linux/cports/pull/165/commits/1e92a0370909a890f03252ed61ae81266d5b34b3), perhaps something else changed. Full native build log of that ver fwiw https://paste.c-net.org/zvzhhipyb0rd

munix9 commented 12 months ago

I created a local build for openSUSE Tumbleweed and completely removed the requirements for the created vendor archive and the export of the GOFLAGS variable and it compiled without problems.

So, yes, I also think that is no longer needed for the build.

Biswa96 commented 12 months ago

So, yes, I also think that is no longer needed for the build.

Would like to explain why this is no longer needed? At first, you mentioned that vendored go libraries are required when network is disabled.

munix9 commented 12 months ago

So, yes, I also think that is no longer needed for the build.

Would like to explain why this is no longer needed? At first, you mentioned that vendored go libraries are required when network is disabled.

Until version 34.0.1 that was also necessary. With version 34.0.4 something was changed in the configuration and no more external go-modules are requested.

I test this in a kvm environment without network and have just repeated this: version 34.0.1 without network, vendor-archive and GOFLAGS fails, version 34.0.4 however runs through.

munix9 commented 12 months ago

I'm still testing this under Leap 15.4/5/6 and will let you know.

The builds of version 34.0.4 on the OBS (Open Build Service) are all successful, without vendor-archive and "-mod=vendor" (and without network).

Without a closer look at the differences between 34.0.1 and 34.0.4, I can't say why.

So this PR is not necessary for version 34.0.4, but it doesn't hurt, because I did the first tests with vendor-archiv and they worked without problems.

Biswa96 commented 12 months ago

I have built this project using git repository and source tarball from release. In both cases, go executable downloads modules in $GOCACHE directory. So, I decided to keep things as is.