Add support for opensuse #3298

mbgg · 2017-06-26T16:18:40Z

Know issues:

adhd compilation with gcc7 is broken. Patch to fix this is submitted. As a workaround we would need to pass --Wno-error=int-in-bool-context (this holds for tumbleweed, leap 42.2 should work without a problem).
no audio working, need investigation

normal instalation should work without problems:
sudo sh crouton -r tumbleweed -n test -t xiwi,xfce,keyboard,cli-extra
sudo sh crouton -r leap-42.2 -n test -t xiwi,xfce,keyboard,cli-extra

dnschneid

This is a really great start; thanks! Maybe this will beat gentoo and arch :)

The O of croagh!

dnschneid · 2017-06-27T03:55:47Z

installer/openSUSE/debootstrap/debootstrap.sh

+#
+# You should have received a copy of the GNU General Public License
+# A copy of the GNU General Public License can be downloaded from
+# http://www.gnu.org/licenses/gpl.html


We can't include GPL'd code in crouton, unfortunately. Why do you need to emulate debootstrap's semantics, anyway?

You'll need to delete this code from the commit history before the pull request can be merged.

Ok, I'll see how to resolve this.

dnschneid · 2017-06-27T03:57:12Z

host-bin/enter-chroot

+        systemd_logind="/usr/lib/systemd/systemd-logind"
+    fi
+
+    if [ -x "$CHROOT${systemd_logind}" ] && \


nit: don't need { } here and in other places

dnschneid · 2017-06-27T03:58:36Z

installer/openSUSE/bootstrap

+# directory), $INSTALLERDIR/$DISTRO, $RELEASE, $BOOTSTRAP_RELEASE (if different
+# from $RELEASE), $ARCH, and $MIRROR.
+
+CUR=`pwd`


dnschneid · 2017-06-27T03:59:24Z

installer/openSUSE/defaults

+# It must set at least ARCH and MIRROR if not already specified.
+
+if [ -z "$ARCH" ]; then
+    ARCH="`uname -m`"


use $( ) instead (here and elsewhere).

Yes, the code you're referencing still has it, but we're moving away from that on all new/modified code.

dnschneid · 2017-06-27T04:00:45Z

installer/openSUSE/defaults

+esac
+
+if [ -z "$MIRROR" ]; then
+    MIRROR="${CROUTON_MIRROR_opensuse_arm:-http://download.opensuse.org/ports/}"


Is this the right mirror for x86?

No, but actually MIRROR is not used anywhere. I'll delete it.

dnschneid · 2017-06-27T04:29:21Z

targets/xorg

@@ -43,7 +43,7 @@ inteldriver=''
 intelhasfbc=''
 pinmesa=''
 intelfbcsupport='y'
-if [ "${ARCH#arm}" = "$ARCH" ]; then
+if [ "${ARCH#arm}" = "$ARCH" ] && [ "${ARCH#aarch64}" = "$ARCH" ]; then


oh, this looks like a bug...could you make a separate PR for this? I wonder if we've done similar mistakes elsewhere.

@drinkcat

...or is it just that debian/ubuntu refers to it as arm64?

Well in install/debian/defaults we set ARCH to arm64 while I did set it to aarch64. We should unify this. I tend to use aarch64, as arm64 is only a kernel thing, but I don't care too much honestly.

let's be consistent with the other distros and use arm64

ok, I'll fix installer/opensuse/getrelease.sh

dnschneid · 2017-06-27T04:31:07Z

targets/xorg

@@ -68,7 +68,7 @@ if [ "${ARCH#arm}" = "$ARCH" ]; then
 fi

 # Catalog relevant and irrelevant video drivers
-fbdev="xserver-xorg-video-fbdev$backport"
+fbdev="openSUSE=xf86-video-fbdev,xserver-xorg-video-fbdev$backport"


I wonder if it would be cleaner to have each distro set the xorg package name prefix at the top and then just reference $xorg-video-fbdev (etc) without needing any distro modifiers

I don't see any schema that would be easy to implement. Some have a prefix, some have a postfix and some have a different name.

dnschneid · 2017-06-27T04:31:44Z

targets/xorg

@@ -97,7 +97,7 @@ fi

 # On Freon, we need crazy xorg hacks
 if [ -n "$freon" ]; then
-    compile freon '-ldl -ldrm -I/usr/include/libdrm' so libdrm-dev
+    compile freon '-ldl -ldrm -I/usr/include/libdrm' so openSUSE=libdrm-devel,libdrm-dev


same goes for devel vs. dev, I guess. You don't have to make any changes, though.

dnschneid · 2017-06-27T04:32:16Z

targets/xorg

@@ -181,7 +181,7 @@ if [ -n "$backport" ]; then
        "xserver-xorg-input-synaptics$backport" $installvideodrivers
 fi

-if release -lt stretch -lt kali-rolling -lt yakkety; then
+if release -lt stretch -lt kali-rolling -lt yakkety -lt tumbleweed; then


-lt tumbleweed means any opensuse release that is earlier than tumbleweed, not including tumbleweed itself. Is that what you meant?

note that if you don't want this for any opensuse release, you can just leave the line unmodified (without -lt tumbleweed) and it won't match opensuse.

dnschneid · 2017-06-27T04:33:11Z

targets/xorg

@@ -194,12 +194,12 @@ if [ -z "$inteldriver" -a -n "$backport" ] && release -eq precise; then
    # so replace it with something not broken
    install_dummy xorg -- xserver-xorg$backport libgl1-mesa-glx$backport \
        libgl1-mesa-dri$backport libglu1-mesa xfonts-base x11-apps \
-        x11-session-utils x11-utils x11-xfs-utils x11-xkb-utils \
+        x11-session-utils openSUSE=xprop,x11-utils x11-xfs-utils x11-xkb-utils \


is xprop the only thing we need from x11-utils?

yes, AFAIK it is the only tool needed

googlebot · 2017-07-03T08:53:08Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

googlebot · 2017-07-03T09:01:59Z

CLAs look good, thanks!

mbgg · 2017-07-10T09:30:00Z

Just a friendly ping that I send a new pull request in here (not sure if I screwed up because we lost the comments on the old one). So any comments are welcome :)

dnschneid · 2017-07-14T05:58:06Z

installer/opensuse/debootstrap.sh

@@ -0,0 +1,76 @@
+#!/bin/sh -e
+# Copyright (c) 2017 The crouton Authors. All rights reserved.


You can't claim copyright just by deleting code, and most (all?) of this file appears to be copied from here.

There's also no reason to fake the interface of debootstrap. Please make an effort to implement opensuse/bootstrap without the use or reference of this code. I suspect you can write what you need from scratch in a few lines inside opensuse/bootstrap.

mbgg · 2017-07-26T12:07:32Z

I updated the pull request. I dropped debootstrap and implemented the necessary bits directly in bootstrap. Also I added two commits for chromium (un)support.

Happly awaiting any comments :)

dnschneid · 2017-07-26T22:02:50Z

installer/opensuse/bootstrap

+# from $RELEASE), $ARCH, and $MIRROR.
+
+# Tumbleweed
+url_tumbleweed_armv7hl="http://download.opensuse.org/ports/armv7hl/tumbleweed/images/openSUSE-Tumbleweed-ARM-JeOS.armv7hl-rootfs.armv7l-Current.tbz"


how did you decide on these URLs? The sizes are dramatically different (the X11 ones are 800 megs!). There're also distribution/openSUSE-current/appliances/ and distribution/openSUSE-stable/appliances/ directories with similar images.

There are unversioned entries in the leap subdirectory. Are those always updated? Are we going to have to keep updating the tarball in this file if we specify a version?

And what happens on i386/x86_64?

JeOS images are minimal images (Just Enough Operating System). For armv7hl no plain JeOS image exists. I'm investigating this. Maybe we can just delete the armv7hl support for Leap42.2 and add it in a later patch.

Leap images don't get updated, they get released and that's it. The unversioned images are links to the versioned ones. I'll update the links, as the version is of no use.

We should try to add a automatic resolution of architecutre + os-version -> download link, but I would prefer to leave this for a latter patch, e.g. when adding Leap-42.3 support. TBH I'm not too confident that this will work out, as the folder structure of the download server might change for newer releases.

i386/x86_64 images don't exists at this point in time, only the installer ISO is created.

so as we can't support i386 right now, I will delete the architecutre from installer/opensuse/defaults as well.

Ok, armv7hl JeOS image got created now, I will fix the URL.

dnschneid · 2017-07-26T22:03:33Z

installer/opensuse/bootstrap

+BOOTSTRAP_RELEASE="$RELEASE"
+
+# Rename factory to tumbleweed
+[ "$RELEASE" = "factory" ] && BOOTSTRAP_RELEASE=tumbleweed


please convert this to a full if statement

dnschneid · 2017-07-26T22:04:55Z

installer/opensuse/bootstrap

+[ "$ARCH" = "arm64" ] && BOOTSTRAP_ARCH=aarch64
+
+key=$(echo "\$url_${BOOTSTRAP_RELEASE}_${BOOTSTRAP_ARCH}")
+URL=$(echo $(eval echo "${key}"))


I see what you're trying to do here, but considering all the setup and checking necessary, is this really shorter than just using if statements to explicitly set the URL?

The idea behind that is, that if we can't be sure the download links keep the same folder structure over time, we just add a new identifier for, let's say leap-42.3 and we are fine.

This means that we need to convert "leap-42.3" to "leap_42_3" which is done in a dump way up to now. For the next revision I will change this using sed. So in effect if a new Leap release is added, we just need to put url_leap_42_3_aarch64 URL in the file and we are settled. This is still worse then other distributions in crouton, but IMHO better then start adding if statements of every new release. Thoughts?

A new if statement (or a case statement) would probably be easier to follow, but maybe there's some complexity in that approach I'm not seeing.

I think with this approach, adding a new version/architecture is easier, as it is a one-line add.
When you put that in if statements, it's more complicated, as you have to figure out between different if's, the arch and distribution name.

How about building up the URL in pieces?

Ok, I hacked that up for you, let me know if you prefer this, I definitely do not. File path and file names are not homogenous on architectures and releases. I expect this to break with a new architecture. Apart I'm quite sure that if you don't have the actual links in front of you, it is really hard to understand how this links will look like:

BOOTSTRAP_ARCH="$ARCH" BOOTSTRAP_RELEASE="$RELEASE" CURRENT="" if [ "$ARCH" = "arm64" ] || [ "$ARCH" = "aarch64" ]; then BOOTSTRAP_ARCH="aarch64" BOOTSTRAP_ARCH2="aarch64" BOOTSTRAP_ARCH3="aarch64" elif [ "$ARCH" = "armv7hl" ]; then BOOTSTRAP_ARCH="armv7hl" BOOTSTRAP_ARCH2="armv7hl" BOOTSTRAP_ARCH3="armv7l" fi if [ "$RELEASE" = "factory" ] || [ "$RELEASE" = "tumbleweed" ]; then BOOTSTRAP_VERSION="." BOOTSTRAP_RELEASE="tumbleweed/images" BOOTSTRAP_RELEASE2="." BOOTSTRAP_RELEASE3="Tumbleweed" CURRENT="-Current" fi case "$RELEASE" in leap*) BOOTSTRAP_VERSION=${RELEASE#leap-} BOOTSTRAP_RELEASE="distribution/leap" BOOTSTRAP_RELEASE2="appliances" BOOTSTRAP_RELEASE3="Leap$BOOTSTRAP_VERSION" if [ "$ARCH" = "armv7hl" ]; then BOOTSTRAP_ARCH2="armv7" fi esac URL="http://download.opensuse.org/ports/$BOOTSTRAP_ARCH/$BOOTSTRAP_RELEASE/$BOOTSTRAP_VERSION/$BOOTSTRAP_RELEASE2/openSUSE-$BOOTSTRAP_RELEASE3-ARM-JeOS.$BOOTSTRAP_ARCH2-rootfs.$BOOTSTRAP_ARCH3$CURRENT.tbz"

So for the record, here are the links we are talking about up to now:
http://download.opensuse.org/ports/armv7hl/tumbleweed/images/openSUSE-Tumbleweed-ARM-JeOS.armv7hl-rootfs.armv7l-Current.tbz
http://download.opensuse.org/ports/aarch64/tumbleweed/images/openSUSE-Tumbleweed-ARM-JeOS.aarch64-rootfs.aarch64-Current.tbz

http://download.opensuse.org/ports/armv7hl/distribution/leap/42.2/appliances/openSUSE-Leap42.2-ARM-JeOS.armv7-rootfs.armv7l.tbz
http://download.opensuse.org/ports/aarch64/distribution/leap/42.2/appliances/openSUSE-Leap42.2-ARM-JeOS.aarch64-rootfs.aarch64.tbz
http://download.opensuse.org/ports/aarch64/distribution/leap/42.3/appliances/openSUSE-Leap42.3-ARM-JeOS.aarch64-rootfs.aarch64.tbz

Haha alright, I'm convinced. Apologies for making you go through that.

No problem, the important thing is, that we reached consens on this :)

dnschneid · 2017-07-26T22:05:08Z

installer/opensuse/bootstrap

+    exit 1
+fi
+
+mkdir -p $tmp/$subdir


dnschneid · 2017-07-26T22:05:46Z

installer/opensuse/bootstrap

+fi
+
+mkdir -p $tmp/$subdir
+( cd $tmp/$subdir; wget -O - $URL | bzip2 -d | tar x ) 1>&2


quoting. Also, bzip2 -d | tar x could just be tar -jx

wget is being removed from the rootfs, so use curl:

curl -# -L --connect-timeout 60 --retry 2 "$URL" | tar -jx -C "$tmp/$subdir"

dnschneid · 2017-07-26T22:06:05Z

installer/opensuse/bootstrap

+mkdir -p $tmp/$subdir
+( cd $tmp/$subdir; wget -O - $URL | bzip2 -d | tar x ) 1>&2
+
+echo 'Remove /dev files as not needed'


I don't think the user needs to see this

dnschneid · 2017-07-26T22:06:24Z

installer/opensuse/bootstrap

+( cd $tmp/$subdir; wget -O - $URL | bzip2 -d | tar x ) 1>&2
+
+echo 'Remove /dev files as not needed'
+rm -rf ${tmp}/${subdir}/dev/*


quoting. you also don't need braces around the variable names

dnschneid · 2017-07-26T22:06:41Z

installer/opensuse/defaults

+# It must set at least ARCH and MIRROR if not already specified.
+
+if [ -z "$ARCH" ]; then
+    ARCH="`uname -m`"


dnschneid · 2017-07-26T22:07:41Z

installer/opensuse/getrelease.sh

+
+# Create release name from /etc/os-release file
+# sed specialist can make that more elegant...
+rel=$(grep CPE_NAME "$sources" | cut -d ":" -f 4)


dnschneid · 2017-07-26T22:09:12Z

installer/opensuse/getrelease.sh

+# Create release name from /etc/os-release file
+# sed specialist can make that more elegant...
+rel=$(grep CPE_NAME "$sources" | cut -d ":" -f 4)
+if [ $rel = "opensuse" ]; then


going to stop pointing it out here...but you need to make sure all your variable expansions are properly quoted

ok, hopefully I will catch all of them.

dnschneid · 2017-07-26T22:10:42Z

installer/opensuse/prepare

+        pkgsasdeps="`list_uninstalled_dist '' $pkgs`"
+    fi
+    zypper -q install -y $params $pkgs `list_uninstalled_dist - "$@"` || return $?
+# TODO see how we can implement that reasonable with zypper


what's the plan here?

There is none such functionality in zypper, I think we should get rid of all the asdeps stuff in this file. Thoughts?

Sure; the resulting chroot will just be larger because of it.

dnschneid · 2017-07-26T22:11:07Z

installer/opensuse/prepare

+        fi
+        shift
+    done
+    while [ ! "$#" = 0 ]; do


dnschneid · 2017-07-26T22:11:19Z

installer/opensuse/prepare

+        params='--no-recommends'
+        shift
+    fi
+    if [ ! "$#" = 0 ]; then


dnschneid · 2017-07-26T22:11:59Z

installer/opensuse/prepare

+
+%files
+EOF
+    rpmbuild --define='_rpmdir ${tmp}' -bb ${tmp}/dummy.spec > ${tmp}/build.log


quoting. also, is single quotes correct for --define?

yes, man page shows single quotes as well.

dnschneid · 2017-07-26T22:12:13Z

installer/opensuse/prepare

+%files
+EOF
+    rpmbuild --define='_rpmdir ${tmp}' -bb ${tmp}/dummy.spec > ${tmp}/build.log
+    if [ $? != 0 ]


if [ ]; then

dnschneid · 2017-07-26T22:12:50Z

installer/opensuse/prepare

+    done
+}
+
+if [ -r /debootstrap.sh ]; then


not needed?

no, not needed.

dnschneid · 2017-07-26T22:13:54Z

installer/opensuse/prepare

+chmod u+s /usr/bin/ping
+
+# add rpmbuild for dummy packages
+sudo zypper -q install -y --no-recommends rpmbuild


no need for sudo. also, shouldn't this be an install rpmbuild command in core?

dnschneid · 2017-07-26T22:14:39Z

targets/chromium

        [ "${ARCH#arm}" != "$ARCH" ]; then
-    error 99 "chromium target is not supported on Debian/ARM."
+    error 99 "chromium target is not supported on "$DISTRO"/"$ARCH"."


this quoting isn't quite what you want

is it really not available though? seems like it exists, although I have no idea what it involves: https://software.opensuse.org/package/chromium

It's only available for x86, not for ARM.

would something like this be acceptable for you:
error 99 "chromium target is not supported on "$DISTRO"/ARM."

you don't need the quotes around $DISTRO, since the entire string is quoted

and I'd prefer "chromium target is not supported on $DISTRO on ARM"

sure, no problem

dnschneid · 2017-07-26T22:16:35Z

targets/x11-common

@@ -7,6 +7,12 @@
 # forms of X11.

 ### Append to prepare.sh:
+
+#we need to install xinit to be sure we overwrite the path


what does this mean

TBH I can't remember, but we need to install xinit, otherwise graphic does not work.
Do you prefer that I add this line instead of the if:
install opensuse=xinit,

Just let me know.

I'd prefer to not install things unnecessarily, so please check what xinit is pulling in when you try to install it here

It's not pulling in anything, no recommends and no sugguests:

linux@localhost:~> zypper info --recommends --suggests xinit Loading repository data... Reading installed packages... Information for package xinit: ------------------------------ Repository : openSUSE-Ports-Leap-42.3-repo-oss Name : xinit Version : 1.3.4-3.1 Arch : aarch64 Vendor : openSUSE Installed Size : 234.6 KiB Installed : Yes (automatically) Status : up-to-date Source package : xinit-1.3.4-3.1.src Summary : X Window System initializer Description : The xinit program is used to start the X Window System server and a first client program on systems that are not using a display manager such as xdm or in environments that use multiple window systems. When this first client exits, xinit will kill the X server and then terminate. Recommends : --- Suggests : ---

Ah, I see. This is fine, then. Just move it under the "remove old vgem hack" section and quote $DISTROAKA

Oh weird, we're already installing xinit in the xorg. Is this an issue for xorg or only for xiwi?

...in which case, xiwi line 31 is probably the right place for it.

ok, I put it at line 31 in xiwi.

dnschneid · 2017-11-16T20:12:08Z

installer/opensuse/bootstrap

+key=$(echo "\$url_${BOOTSTRAP_RELEASE}_${BOOTSTRAP_ARCH}")
+URL=$(echo $(eval echo "${key}"))
+
+if [ ! "$URL" ]; then


mbgg · 2017-11-16T20:35:24Z

@dnschneid , I think there was an overlap between you adding more comments on the code and me updating the pull request. I integrated your last two comments. Please feel free to review :)

dnschneid

Lots of nits and a few minor errors, but no major concerns! Woo!

dnschneid · 2017-11-16T22:10:58Z

installer/opensuse/bootstrap

+fi
+
+# Fix leap release name
+case "$RELEASE" in leap*)


elif [ "${RELEASE#leap}" != "$RELEASE" ]; then

dnschneid · 2017-11-16T22:11:14Z

installer/opensuse/bootstrap

+
+# Fix leap release name
+case "$RELEASE" in leap*)
+     BOOTSTRAP_RELEASE=$(echo "$RELEASE" | sed -e 's/[-.]/_/g')


quoting, and tr would probably be a better choice

dnschneid · 2017-11-16T22:13:35Z

installer/opensuse/bootstrap

+eval "URL=\"\${url_${BOOTSTRAP_RELEASE}_${BOOTSTRAP_ARCH}-}\""
+
+if [ -z "$URL" ]; then
+    echo "Unknown Distribution / Architecture: $key"


if the error function is available, you probably want to use that. Otherwise, you want to redirect to STDERR using >&2

Also, $key doesn't exist anymore

it seems, that no error function is available.

dnschneid · 2017-11-16T22:14:07Z

installer/opensuse/bootstrap

+    exit 1
+fi
+
+mkdir -p "$tmp"/"$subdir"


"$tmp/$subdir"

dnschneid · 2017-11-16T22:15:35Z

installer/opensuse/bootstrap

+fi
+
+mkdir -p "$tmp"/"$subdir"
+( cd "$tmp"/"$subdir"; wget -O - $URL |  tar -jx ) 1>&2


wget is being removed from the rootfs, so use curl:

curl -# -L --connect-timeout 60 --retry 2 "$URL" | tar -jx -C "$tmp/$subdir"

dnschneid · 2017-11-16T22:28:14Z

targets/audio

@@ -99,6 +99,13 @@ build_cras() {
            ln -sfT libasound.so.2 "$libasoundso"
        fi
        ALSALIBDIR="/usr/lib$archextrapath/alsa-lib"
+    elif [ "$DISTROAKA" = "opensuse" ]; then
+        install --minimal --asdeps alsa-devel
+        if [ "$ARCH" = "arm64" ] || [ "$ARCH" = "x86_64"]; then


] || [ should be -o

dnschneid · 2017-11-16T22:28:52Z

targets/chromium

@@ -3,9 +3,9 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 if [ "${TARGETNOINSTALL:-c}" = 'c' ] &&
-        [ "$DISTRO" = 'debian' -o "$DISTRO" = 'kali' ] &&
+        [ "$DISTRO" = 'debian' -o "$DISTRO" = 'kali' -o "$DISTRO" = 'opensuse' ] &&


not distroaka?

dnschneid · 2017-11-16T22:29:18Z

targets/gtk-extra

@@ -8,17 +8,19 @@ DESCRIPTION='GTK-based tools including gdebi, gksu, and a simple browser.'

 ### Append to prepare.sh:
 install_dummy network-manager network-manager-gnome
-install gdebi gksu
+    install opensuse=,gdebi opensuse=libgnomesu,gksu


indentation

dnschneid · 2017-11-16T22:30:18Z

targets/xiwi

+
+# We need to install xinit to make graphics work
+if [ $DISTROAKA = "opensuse" ]; then
+    install xinit


Just add it as opensuse=xinit, in the install line below

dnschneid · 2017-11-16T22:31:06Z

targets/xorg

        x11-xserver-utils xauth xinit xfonts-utils xkb-data xorg-docs-core \
        xterm x11-common xinput
 else
-    install xorg $installvideodrivers -- xserver-xorg-video-all$backport \
+    install opensuse=xorg-x11-server,xorg $installvideodrivers -- opensuse=xorg-x11-driver-video,xserver-xorg-video-all$backport \


probably need to break the line

googlebot · 2017-11-21T16:09:27Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

Signed-off-by: Matthias Brugger <mbrugger@suse.com>

mbgg · 2017-11-21T16:16:49Z

Hi @dnschneid just now the newest version. Feel free to comment :)

mbgg · 2017-12-05T10:24:26Z

Gentle ping, would be nice if you find a moment to review :-)

mbgg · 2018-01-12T17:43:56Z

any further comments on that?

dnschneid · 2018-02-16T00:27:28Z

installer/opensuse/bootstrap

+if [ "$RELEASE" = "factory" ]; then
+    BOOTSTRAP_RELEASE=tumbleweed
+elif [ "${RELEASE#leap}" != "$RELEASE" ]; then
+     BOOTSTRAP_RELEASE=$(echo "$RELEASE" | tr - _ | tr . _)


quoting (and line below)

dnschneid · 2018-02-16T00:28:28Z

installer/opensuse/defaults

+case "$ARCH" in
+arm*) ARCH="armv7hl";;
+arm64 | aarch64) ARCH="arm64";;
+*) error 2 "Invalid architecture '$ARCH'.";;


definitely need x86 support before we can merge; this would confuse most of our users

dnschneid

Sorry I'm so slow at looking at this, especially since you've done such a great job cleaning it up.

It would be helpful if you avoid squashing fixes back into old commits (maybe until it's approved for merge and then you can rebase and make it pretty), since that causes the Github review UI to drop most of the history and comments and results in me needing to do a full review every time you update.

dnschneid · 2018-02-16T00:30:17Z

installer/opensuse/prepare

+    rpmbuild --define='_rpmdir "$tmp"' -bb "$tmp/dummy.spec" > "$tmp/build.log"
+    if [ $? != 0 ]; then
+        echo "ERROR: Could not build dummy rpm!"
+	exit 1


spaces, not tabs

dnschneid requested changes Jun 27, 2017

View reviewed changes

mbgg force-pushed the opensuse branch from 5b4675b to 2616aae Compare July 3, 2017 08:53

mbgg force-pushed the opensuse branch 2 times, most recently from 238fd08 to 60145c0 Compare July 3, 2017 09:01

dnschneid reviewed Jul 14, 2017

View reviewed changes

mbgg force-pushed the opensuse branch 2 times, most recently from 7e8d768 to 303d26f Compare July 26, 2017 11:59

dnschneid reviewed Jul 26, 2017

View reviewed changes

installer/opensuse/bootstrap Outdated

exit 1

fi

mkdir -p $tmp/$subdir

Copy link

Owner

dnschneid Jul 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quoting

mbgg reacted with thumbs up emoji

dnschneid reviewed Jul 26, 2017

View reviewed changes

installer/opensuse/prepare Outdated

fi

shift

done

while [ ! "$#" = 0 ]; do

Copy link

Owner

dnschneid Jul 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!=

mbgg reacted with thumbs up emoji

dnschneid reviewed Jul 26, 2017

View reviewed changes

dnschneid reviewed Nov 16, 2017

View reviewed changes

mbgg force-pushed the opensuse branch 2 times, most recently from a30b6ad to db419f9 Compare November 16, 2017 20:33

dnschneid requested changes Nov 16, 2017

View reviewed changes

mbgg force-pushed the opensuse branch from db419f9 to 9a3a1f6 Compare November 21, 2017 16:09

mbgg added 16 commits November 21, 2017 17:11

installer: Add openSUSE distribution

9d117d7

Signed-off-by: Matthias Brugger <mbrugger@suse.com>

targets: post-common: openSUSE needs different groups

6c0c10f

Signed-off-by: Matthias Brugger <mbrugger@suse.com>

targets: core: Enable openSUSE

39bbc6a

Signed-off-by: Matthias Brugger <mbrugger@suse.com>

targets: cli-extra: Enable openSUSE

c2d64ca

Signed-off-by: Matthias Brugger <mbrugger@suse.com>

targets: x11-common: Enable openSUSE

85bc5e0

Signed-off-by: Matthias Brugger <mbrugger@suse.com>

targets: audio: Enable openSUSE

8a380c8

Signed-off-by: Matthias Brugger <mbrugger@suse.com>

targets: xorg: Enable openSUSE

65a785e

Signed-off-by: Matthias Brugger <mbrugger@suse.com>

targets: extension: Enable openSUSE

963eb92

Signed-off-by: Matthias Brugger <mbrugger@suse.com>

targets: gtk-extra: Enable openSUSE

e664c13

Signed-off-by: Matthias Brugger <mbrugger@suse.com>

targets: xiwi: Enable openSUSE

dff5a88

Signed-off-by: Matthias Brugger <mbrugger@suse.com>

targets: xfce: Enable openSUSE

98ab387

Signed-off-by: Matthias Brugger <mbrugger@suse.com>

targets: gnome: Enable openSUSE

2c1da7d

Signed-off-by: Matthias Brugger <mbrugger@suse.com>

host-bin: enter-chroot: Enable systemd for openSUSE

0f8c8ca

Signed-off-by: Matthias Brugger <mbrugger@suse.com>

chromium: Print distribution in error message

79472fb

Signed-off-by: Matthias Brugger <mbrugger@suse.com>

chromium: openSUSE does not support chromium

3cc2fc1

Signed-off-by: Matthias Brugger <mbrugger@suse.com>

installer: opensuse: add Leap 42.3

9d68d80

Signed-off-by: Matthias Brugger <mbrugger@suse.com>

mbgg force-pushed the opensuse branch from 9a3a1f6 to 9d68d80 Compare November 21, 2017 16:14

dnschneid reviewed Feb 16, 2018

View reviewed changes

dnschneid requested changes Feb 16, 2018

View reviewed changes

dnschneid added the stale label Dec 31, 2021

		@@ -0,0 +1,76 @@
		#!/bin/sh -e
		# Copyright (c) 2017 The crouton Authors. All rights reserved.

Add support for opensuse #3298

Are you sure you want to change the base?

Add support for opensuse #3298

Conversation

mbgg commented Jun 26, 2017 • edited

dnschneid left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

googlebot commented Jul 3, 2017

googlebot commented Jul 3, 2017

mbgg commented Jul 10, 2017

Choose a reason for hiding this comment

mbgg commented Jul 26, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbgg Aug 4, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbgg Nov 16, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbgg commented Jun 26, 2017 •

edited

mbgg Aug 4, 2017 •

edited

mbgg Nov 16, 2017 •

edited