Debian packaging #609

hysw · 2025-11-27T23:00:12Z

No description provided.

hysw · 2025-12-12T00:02:32Z

This is a partial solution to automated linux packaging.

scripts/package_linux.sh

CMakeLists.txt

packaging/linux/CMakeLists.txt

footballhead · 2025-12-12T15:25:53Z

scripts/package_linux.sh

@@ -0,0 +1,56 @@
+#!/bin/bash


Consider marking this script as executable

footballhead · 2025-12-12T15:28:29Z

scripts/package_linux.sh

+mkdir -p build/package_linux/host
+# /home/runner/work/dive/Qt
+pushd build/package_linux/device
+    cmake  -DCMAKE_TOOLCHAIN_FILE=${ANDROID_NDK_HOME}/build/cmake/android.toolchain.cmake \


DRY. consider invoking build_android.sh instead to reduce the number of copies and improve maintainability

build_android.sh script is quite a bit different than the scripts here. They overlap in CMake options, and that's about it.

We probably should have a cmake preset.

That's a good idea

scripts/package_linux.sh

.github/workflows/presubmit.yml

hysw · 2025-12-16T16:08:15Z

.github/workflows/presubmit.yml

+          java-version: '17'
+          distribution: 'temurin'
+      - name: Check out code
+        uses: actions/checkout@v3


Update github action pinned version.

footballhead · 2025-12-16T16:14:15Z

scripts/package_linux.sh

+
+readonly DIVE_ROOT="$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )/.." &> /dev/null && pwd )"
+
+mkdir -p build/package_linux/device


Should this be explicitly prefixed with DIVE_ROOT for consistent behavior or did you want to allow the user to change this based on their current working directory?

footballhead · 2025-12-16T16:15:53Z

scripts/package_linux.sh

+        -DCMAKE_FIND_ROOT_PATH_MODE_INCLUDE=NEVER \
+        -DCMAKE_FIND_ROOT_PATH_MODE_LIBRARY=NEVER \
+        -DDIVE_GFXR_GRADLE_CONSOLE=plain \
+        ${DIVE_ROOT} || exit 1


the exits are redundant given the set -e at the top

footballhead · 2025-12-17T15:28:54Z

scripts/package_linux.sh

+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -ex


nit: set -u will halt for undefined variables. This can catch if e.g. ANDROID_NDK_HOME is not set

Suggested change

set -ex

set -eux

footballhead · 2025-12-17T15:29:27Z

scripts/package_linux.sh

+    pushd "${DIVE_BUILD_ROOT}/package_linux/device"
+        cmake -DCMAKE_TOOLCHAIN_FILE=${ANDROID_NDK_HOME}/build/cmake/android.toolchain.cmake \
+            -G "Ninja" \
+            -DCMAKE_MAKE_PROGRAM="ninja" \


nit: Given that the generator is Ninja, I don't think we need to explicitly set the make program as well

Suggested change

-DCMAKE_MAKE_PROGRAM="ninja" \

footballhead · 2025-12-17T15:33:25Z

scripts/package_linux.sh

+            -DCMAKE_FIND_ROOT_PATH_MODE_LIBRARY=NEVER \
+            -DDIVE_GFXR_GRADLE_CONSOLE=plain \
+            ${DIVE_ROOT}
+        cmake --build . --config=Debug -j


nit: is -j necessary with ninja? I think "all the jobs!" is the ninja default

Also since we're not using a multi-config generator, do we need --config?

Suggested change

cmake --build . --config=Debug -j

cmake --build .

footballhead · 2025-12-17T15:34:50Z

scripts/package_linux.sh

+    cmake --build . --config Release
+    cmake --install . --prefix ../dive_linux --config Release


since we're not using the multi-config generator, do we need --config?

Suggested change

cmake --build . --config Release

cmake --install . --prefix ../dive_linux --config Release

cmake --build .

cmake --install . --prefix ../dive_linux

footballhead · 2025-12-17T15:36:00Z

packaging/CMakeLists.txt

+if(EXISTS "${DIVE_ANDROID_PREBUILT}")
+    install(
+        DIRECTORY "${DIVE_ANDROID_PREBUILT}/"
+        # TODO: fix device resources location.


prefer TODO with bugs so that future readers have more context

footballhead · 2025-12-17T15:37:25Z

.github/workflows/presubmit.yml

          cat build/.ninja_log
+      - name: Produce install artifacts
+        run: |
+          cmake --install build --prefix build/install_artifact --config Release


nit: since we're not using the multi-config genreator, do we need --config? Also, this is a debug build

Suggested change

cmake --install build --prefix build/install_artifact --config Release

cmake --install build --prefix build/install_artifact

Need to be updated according to #679

footballhead · 2025-12-17T15:40:09Z

.github/workflows/presubmit.yml

+          touch build/package_linux/dive_android_is_prebuilt
+          bash scripts/package_linux.sh


optional: consider making dive_android_is_prebuilt an option for package_linux.sh so that it's more obvious to callers about the behavior they're going to get. It also lets the caller more easily choose behavior

footballhead · 2025-12-17T15:40:51Z

.github/workflows/presubmit.yml

+        run: ls -R build/package_linux/dive_android
+      - name: Build
+        run: |
+          mkdir -p build/package_linux/


nit: since you downloaded build/package_linux, do you need to mkdir?

Suggested change

mkdir -p build/package_linux/

wangra-google · 2025-12-19T03:36:18Z

a more general question, we are currently saving capture files, or even crash dump file (https://github.com/google/dive/pull/638/changes#diff-ecc0a1f137a1e178eb72d83ee5750d7f896705149afb8c3fab743c2a05e97952R106) at the same folder as executables, would that be a problem with your change since the user may not have write permission with the executable install folder?

hysw · 2025-12-19T19:48:45Z

a more general question, we are currently saving capture files, or even crash dump file at the same folder as executables, would that be a problem with your change since the user may not have write permission with the executable install folder?

That's a related but separate question. We should follow platform idiom for those, e.g. probably ${HOME}/.dive-profiler on Linux.

hysw marked this pull request as draft November 27, 2025 23:00

hysw force-pushed the dpkg branch 4 times, most recently from 85bd214 to ccee0ab Compare November 28, 2025 01:01

hysw force-pushed the dpkg branch from ccee0ab to 4255fd0 Compare December 10, 2025 21:26

hysw mentioned this pull request Dec 10, 2025

Improve CMake install semantics. #652

Merged

hysw force-pushed the dpkg branch 2 times, most recently from eae670e to 27bf71f Compare December 12, 2025 00:00

hysw requested review from angela28chen, elviscapiaq, footballhead and wangra-google December 12, 2025 00:01

hysw marked this pull request as ready for review December 12, 2025 00:01

hysw force-pushed the dpkg branch from 27bf71f to b7fa5d9 Compare December 12, 2025 00:24

hysw commented Dec 12, 2025

View reviewed changes

scripts/package_linux.sh Outdated Show resolved Hide resolved

wangra-google previously approved these changes Dec 12, 2025

View reviewed changes

footballhead reviewed Dec 12, 2025

View reviewed changes

hysw closed this Dec 15, 2025

hysw force-pushed the dpkg branch from b7fa5d9 to 7bf45f1 Compare December 15, 2025 22:37

hysw reopened this Dec 15, 2025

hysw dismissed wangra-google’s stale review via b871851 December 15, 2025 22:38

hysw commented Dec 16, 2025

View reviewed changes

footballhead reviewed Dec 16, 2025

View reviewed changes

hysw added 3 commits December 16, 2025 13:16

Debian Packaging.

65dc05a

Workflow chain.

e565e07

Address comments.

cd399bf

hysw force-pushed the dpkg branch from b871851 to cd399bf Compare December 16, 2025 18:27

wangra-google approved these changes Dec 17, 2025

View reviewed changes

footballhead reviewed Dec 17, 2025

View reviewed changes

wangra-google mentioned this pull request Dec 19, 2025

Adds Crashpad as a submodule from Backtrace Labs fork #638

Merged


		readonly DIVE_ROOT="$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )/.." &> /dev/null && pwd )"

		mkdir -p build/package_linux/device

		cmake --build . --config Release
		cmake --install . --prefix ../dive_linux --config Release

	cmake --install build --prefix build/install_artifact --config Release
	cmake --install build --prefix build/install_artifact

		touch build/package_linux/dive_android_is_prebuilt
		bash scripts/package_linux.sh

Debian packaging #609

Are you sure you want to change the base?

Debian packaging #609

Uh oh!

Conversation

hysw commented Nov 27, 2025

Uh oh!

hysw commented Dec 12, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wangra-google commented Dec 19, 2025

Uh oh!

hysw commented Dec 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants