Date: Thu, 15 Oct 2015 09:06:04 +0200
From: Natanael Copa <ncopa@alpinelinux.org>
To: =?ISO-8859-1?B?U/ZyZW4=?= Tempel <soeren+git@soeren-tempel.net>
Cc: alpine-aports@lists.alpinelinux.org
Subject: Re: [alpine-aports] [PATCH] main/wpa_supplicant: copy a config file
 based on defconfig
Message-ID: <20151015090604.15e14a13@ncopa-desktop.alpinelinux.org>
In-Reply-To: <1444228684-2361-1-git-send-email-soeren+git@soeren-tempel.net>
References: <1444228684-2361-1-git-send-email-soeren+git@soeren-tempel.net>
Precedence: list
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

On Wed,  7 Oct 2015 16:38:04 +0200
S=F6ren Tempel <soeren+git@soeren-tempel.net> wrote:

> Instead of appending configuration variables to the .config file
> manually. This has the advantage that configuration option can easily be
> adjusted and are well documented in addition to that options that have
> been removed on package upgrade can be identified easier.

Generally, I prefer to use upstream default config from upstream source
package rather than to have a fork of the default config. If upstream
add/remove default config we normally want follow that. Then we only
maintain the changes.

If we still want to fork the config this way, then I think we should do
this in 2 steps. First is make a config that is identical to current
config but just have the new format. Second we implement the changes
from what we currently have til the new stuff, CONFIG_EAP_FAST,
readline, smartcard etc.

The reason is that if we change format and change config in same commit
it becomes very hard to see what as actually changed in case something
break. If we need revert the change due to users being unhappy then we
would end up revert both config format change and config change while
we only would want revert the config change.
=20
> Many options enabled before where no longer mentioned in the defconfig
> file and have thus been removed. The 'CONFIG_EAP_FAST' option has been
> enabled additionally.

As said. config change should be in separate commit as the config
format change commit to make it more visisble what is actually going on
and to avoid unintented config changes.

> I also had to add readline-dev and pcsc-lite-dev to the makedepends.
> readlinee-dev is needed for readline support in wpa_cli and
> pcsc-lite-dev is needed for smartcard support. Both options were
> enabled before.

But it does not seem like current wpa_supplicant was built with either.

I don't mind to add the functionality of readline but I would really
like to avoid GNU readline implementation if possible. There are
libedit, linenoise and it also looks like wpa_supplicant has its own
readline-like implementation.

pcsc-lite-dev is ok though.
=20
> Since this is a rather important package I would suggested that this
> is tested by someone else before merging it into the master branch. I
> tested it here and it works for my setup, which doesn't mean that I
> accidentally broke more complicated setups by failing to copy an
> existing configuration option.

I would like to break up the commit in various minor changes so we can
easily revert if needed. Doing changes in small steps will also make it
easier to figure out what change causes potensial breakages.

...

> --- a/main/wpa_supplicant/wpa_cli.sh
> +++ b/main/wpa_supplicant/wpa_cli.sh
> @@ -9,7 +9,7 @@ if [ -z "${1}" -o -z "${2}" ]; then
>  	logger -t wpa_cli "this script should be called from
> wpa_cli(8)" exit 1
>  elif ! [ -x "${IFUP}" -a -x "${IFDOWN}" ]; then
> -	logger -t wpa_cli "/sbin/ifup or /sbin/ifdown doesn't exist"
> +	logger -t wpa_cli "${IFUP} or ${IFDOWN} doesn't exist"
>  	exit 1
>  fi
> =20

this change looks ok but i'd like it in separate commit with commit
message that correspond to the change.

-nc


---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---