On Tue, Apr 4, 2023 at 5:05 PM Nicolas Schier <nicolas@xxxxxxxxx> wrote:
> >
> > debian: FORCE
> > $(call cmd,debianize)
> > @@ -103,6 +103,7 @@ PHONY += debian-orig
> > debian-orig: private source = $(shell dpkg-parsechangelog -S Source)
> > debian-orig: private version = $(shell dpkg-parsechangelog -S Version | sed 's/-[^-]*$$//')
> > debian-orig: private orig-name = $(source)_$(version).orig.tar.gz
> > +debian-orig: mkdebian-opts = --need-source
> > debian-orig: linux.tar.gz debian
> > $(Q)if [ "$(df --output=target .. 2>/dev/null)" = "$(df --output=target $< 2>/dev/null)" ]; then \
> > ln -f $< ../$(orig-name); \
> > diff --git a/scripts/package/gen-diff-patch b/scripts/package/gen-diff-patch
> > index f842ab50a780..a180ff94f655 100755
> > --- a/scripts/package/gen-diff-patch
> > +++ b/scripts/package/gen-diff-patch
> > @@ -2,43 +2,36 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> >
> > diff_patch="${1}"
> > -untracked_patch="${2}"
> > srctree=$(dirname $0)/../..
> >
> > -rm -f ${diff_patch} ${untracked_patch}
> > -
> > -if ! ${srctree}/scripts/check-git; then
> > - exit
> > -fi
> > -
> > -mkdir -p "$(dirname ${diff_patch})" "$(dirname ${untracked_patch})"
> > +mkdir -p "$(dirname ${diff_patch})"
>
> shellcheck complains about missing quotes around "${diff_patch}".
I will fix it.
>
> >
> > git -C "${srctree}" diff HEAD > "${diff_patch}"
> >
> > -if [ ! -s "${diff_patch}" ]; then
> > - rm -f "${diff_patch}"
> > +if [ ! -s "${diff_patch}" ] ||
> > + [ -z "$(git -C "${srctree}" ls-files --other --exclude-standard | head -n1)" ]; then
>
> Did you leave out the 'rm -f "${diff_patch}"' to have a more static mkspec
> output?
Yes.
I noticed it is ok to leave an empty patch file.
The 'patch' command exists with 0
when the input diff is empty.
$ patch -p1 < /dev/null
$ echo $?
0
However, dpkg-source warns of an empty patch file.
So, mkdebian should remove it when it gets empty.
> > +# The source tarball, which is generated by 'git archive', contains everything
> > +# you committed in the repository. If you have local diff ('git diff HEAD'),
> > +# it will go into ${diff_patch}. If untracked files are remaining, the resulting
> > +# source package may not be correct.
> > +#
> > +# Examples:
> > +# - You modified a source file to add #include <linux/new-header.h>
> > +# but forgot to add include/linux/new-header.h
> > +# - You modified a Makefile to add 'obj-$(CONFIG_FOO) += new-dirver.o'
> > +# but you forgot to add new-driver.c
> > +#
> > +# You need to commit them, or at least stage them by 'git add'.
>
> making the file(s) known to git by 'git add -N' would be sufficient; but that's
> probably too much detail here. Nevertheless, I think the explanation is
> valueable!
Yeah, it is up to users how to remember to do 'git add'.
'git add -N' is a minute.
> > +
> > + # Add .config as a patch
> > + mkdir -p debian/patches
> > + {
> > + echo "Subject: Add .config"
> > + echo "Author: ${maintainer}"
> > + echo
> > + echo "--- /dev/null"
> > + echo "+++ linux/.config"
> > + diff -u /dev/null "${KCONFIG_CONFIG}" | tail -n +3
> > + } > debian/patches/config
> > + echo config > debian/patches/series
>
> I'd named it config.patch, but actually it is just a config, so this makes
> sense to me as well.
I agree.
I will rename it in v2.
>
> > +
> > + $(dirname $0)/gen-diff-patch debian/patches/diff.patch
>
> "${0%/*}" instead of $(dirname $0) would also be possible.
>
> > + if [ -s debian/patches/diff.patch ]; then
> > + echo diff.patch >> debian/patches/series
> > + else
> > + rm -f debian/patches/diff.patch
> > + fi
> > +}
> > +
> > rm -rf debian
> > +mkdir debian
> > +
> > +if [ "$1" = --need-source ]; then
> > + gen_source
> > + shift
>
> Might you want to remove the 'shift'? It looks like mkdebian handles more
> command line arguments but it doesn't, as far as I can see. And in case it
> will do in some future, argument handling had to be revised nevertheless.
OK. I will remove it.
> > echo $debarch > debian/arch
> > extra_build_depends=", $(if_enabled_echo CONFIG_UNWINDER_ORC libelf-dev:native)"
> > extra_build_depends="$extra_build_depends, $(if_enabled_echo CONFIG_SYSTEM_TRUSTED_KEYRING libssl-dev:native)"
> > diff --git a/scripts/package/mkspec b/scripts/package/mkspec
> > index b7d1dc28a5d6..b45020d64218 100755
> > --- a/scripts/package/mkspec
> > +++ b/scripts/package/mkspec
> > @@ -19,8 +19,7 @@ else
> > mkdir -p rpmbuild/SOURCES
> > cp linux.tar.gz rpmbuild/SOURCES
> > cp "${KCONFIG_CONFIG}" rpmbuild/SOURCES/config
> > - $(dirname $0)/gen-diff-patch rpmbuild/SOURCES/diff.patch rpmbuild/SOURCES/untracked.patch
> > - touch rpmbuild/SOURCES/diff.patch rpmbuild/SOURCES/untracked.patch
> > + $(dirname $0)/gen-diff-patch rpmbuild/SOURCES/diff.patch
>
> Possibly change to "${0%/*}/gen-diff-patch", cp. above?
dirname forks a process.
I will change it to ${srctree}/scripts/package/gen-diff-patch
so it works only with a variable expansion.
--
Best Regards
Masahiro Yamada
