Bug#849273: RFS: gemmlowp/0~20170407-g719389f-1 [ITP]

[PaulLiu]

Hi Lumin,

Thanks a lot. The package LGTM. Sponsored. Will be in NEW queue soon.

Yours,
Paul


On Fri, Jun 23, 2017 at 10:45 AM, Lumin  wrote:

> Hi,
>
> On 17 June 2017 at 13:08, Ying-Chun Liu (PaulLiu) 
> wrote:
> >
> > Just a few things left.
> >
> > 1. It is called README.source not README.Source
> >   The 's' should be in lower case.
>
> Renamed.
>
> > 2. I found that the copyright holder should have
> >  "Silk Labs Inc." in AUTHORS file.
> > And also some of the header files show that the copyright holder are
> > "2015 The Gemmlowp Authors"
> > Please also add these to debian/copyright.
>
> Updated copyright.
>
> The updated package was uploaded to mentors
>
> https://mentors.debian.net/debian/pool/main/g/gemmlowp/
> gemmlowp_0~20170609-g36ffd29-1.dsc
>
> as well as debomatic
>
> http://debomatic-amd64.debian.net/distribution#experimental/
> gemmlowp/0~20170609-g36ffd29-1/buildlog
>
> > For tensorflow, if possible, we can just patch the cmake file by quilt
> > and let it built against the libraries in system. I'll try to make a
> > patch. Or perhaps you can tell me where the feature request is and I'll
> > see if I can help.
>
> Yes, we can patch the cmake files.
> The feature request is located here:
>
> https://github.com/tensorflow/tensorflow/issues/9298
>
> But no one seems to be interested in it.
> I attempted to patch the cmake files but it's still a WIP,
> and the patch is attached. Feel free to use it if it's useful.
>
> My idea was to guard those external project cmake files
> with tensorflow_USE_SYSTEM_XXX flags just like what
> Julialang has done.
>
> --
> Best,
> Lumin
>

Bug#849273: RFS: gemmlowp/0~20170407-g719389f-1 [ITP]

[Ying-Chun Liu (PaulLiu)]

On 2017年06月17日 15:48, Lumin wrote:
> Hi,
> 
> On 16 June 2017 at 04:33, Ying-Chun Liu (PaulLiu)  wrote:
>> I've reviewed your package. The package looks quite good to me.
>> The only thing missing is the README.source
>> https://www.debian.org/doc/debian-policy/ch-source.html#s-readmesource
> 
> Updated the package and uploaded to mentors.
> 
> https://mentors.debian.net/debian/pool/main/g/gemmlowp/gemmlowp_0~20170609-g36ffd29-1.dsc
> 
>> The orig.tar.gz seems to be repacked from upstream git repo.
>> Please mention the steps on how you create the orig.tar.gz
>> Like you git clone it. And re-tar it, exclude some directories like .git.
> 
> The orig.tar.gz file is created manually with this command
> 
>   git archive HASH | gzip > xxx.orig.tar.gz
> 
>> Or, provide a way to generate the orig.tar.gz by debian/rules
>> get-orig-source
> 
> Thank you for this snippet, I found many rules file ships such a part
> in its rules file, but it will occupy half of the rules file if I put it in.
> I described how to make the orig.tar.gz file in README.Source.
> 
> Is this acceptable?
> 
> 
> BTW, concerining TensorFlow:
> 
> gemmlowp is likely the last dependency package to be uploaded.
> TensorFlow upstream has cmake build scripts, but those scripts
> will download many other dependency source packages and
> compile them instead of using the pre-compiled ones provided
> by distribution. There is a feature request about this in the
> upstream issue list which is still open.
> 

Hi Lumin,

Just a few things left.

1. It is called README.source not README.Source
  The 's' should be in lower case.

2. I found that the copyright holder should have
 "Silk Labs Inc." in AUTHORS file.
And also some of the header files show that the copyright holder are
"2015 The Gemmlowp Authors"
Please also add these to debian/copyright.


For tensorflow, if possible, we can just patch the cmake file by quilt
and let it built against the libraries in system. I'll try to make a
patch. Or perhaps you can tell me where the feature request is and I'll
see if I can help.

Yours Sincerely,
Paul


-- 
PaulLiu (劉穎駿)
E-mail: Ying-Chun Liu (PaulLiu) 



signature.asc
Description: OpenPGP digital signature

Bug#849273: RFS: gemmlowp/0~20170407-g719389f-1 [ITP]

[Lumin]

Hi,

On 16 June 2017 at 04:33, Ying-Chun Liu (PaulLiu)  wrote:
> I've reviewed your package. The package looks quite good to me.
> The only thing missing is the README.source
> https://www.debian.org/doc/debian-policy/ch-source.html#s-readmesource

Updated the package and uploaded to mentors.

https://mentors.debian.net/debian/pool/main/g/gemmlowp/gemmlowp_0~20170609-g36ffd29-1.dsc

> The orig.tar.gz seems to be repacked from upstream git repo.
> Please mention the steps on how you create the orig.tar.gz
> Like you git clone it. And re-tar it, exclude some directories like .git.

The orig.tar.gz file is created manually with this command

  git archive HASH | gzip > xxx.orig.tar.gz

> Or, provide a way to generate the orig.tar.gz by debian/rules
> get-orig-source

Thank you for this snippet, I found many rules file ships such a part
in its rules file, but it will occupy half of the rules file if I put it in.
I described how to make the orig.tar.gz file in README.Source.

Is this acceptable?


BTW, concerining TensorFlow:

gemmlowp is likely the last dependency package to be uploaded.
TensorFlow upstream has cmake build scripts, but those scripts
will download many other dependency source packages and
compile them instead of using the pre-compiled ones provided
by distribution. There is a feature request about this in the
upstream issue list which is still open.

Bug#849273: RFS: gemmlowp/0~20170407-g719389f-1 [ITP]

[Ying-Chun Liu (PaulLiu)]

On 2017年06月16日 12:33, Ying-Chun Liu (PaulLiu) wrote:
> Hi Lumin,
> 
> I've reviewed your package. The package looks quite good to me.
> The only thing missing is the README.source
> https://www.debian.org/doc/debian-policy/ch-source.html#s-readmesource
> 
> The orig.tar.gz seems to be repacked from upstream git repo.
> Please mention the steps on how you create the orig.tar.gz
> Like you git clone it. And re-tar it, exclude some directories like .git.
> 
> Or, provide a way to generate the orig.tar.gz by debian/rules
> get-orig-source
> 
> For example, added these to debian/rules
> DEB_SOURCE := $(shell dpkg-parsechangelog | grep Source: | sed -e
> 's/Source: //'
> )
> DEB_VERSION := $(shell dpkg-parsechangelog | grep Version: | sed -e
> 's/Version: //')
> DEB_UPSTREAM_VERSION := $(shell echo $(DEB_VERSION) | sed -e 's/-[^-]*$$//')
> GIT_URL = https://github.com/google/gemmlowp.git
> 
> get-orig-source:
> set -e; if echo $(DEB_VERSION) | grep -c "git"; \
> then \
> git_version=`echo $(DEB_VERSION) | sed -e
> 's/^.*~\([0-9]*\)*-\(.*\)-.*$$/\2/g'`; \
> else \
> git_version=$(DEB_UPSTREAM_VERSION); \
> fi; \
> tmpdir=`mktemp -d -t`; \
> cd $$tmpdir; \
> echo "checkout upstream repository ..."; \
> git clone $(GIT_URL); echo "getting specific upstream
> revision/tag: $$git_version"; \
> cd `ls | head -n 1`; git checkout -b orig $$git_version; cd ..; \
> tar --exclude=.git --exclude='*.exe' --exclude='*.jar' -cJvf
> $(CURDIR)/$(DEB_SOURCE)_$(DEB_UPSTREAM_VERSION).orig.tar.xz `ls | head
> -n 1`; \
Sorry, please update this line to

tar --exclude=.git --exclude='*.exe' --exclude='*.jar' -czvf
$(CURDIR)/$(DEB_SOURCE)_$(DEB_UPSTREAM_VERSION).orig.tar.gz `ls | head
-n 1`; \

because you use .tar.gz

> cd $(CURDIR); \
> rm -rf $$tmpdir
> 
> so when developer wants to pack the source tarball from the upstream,
> they can use debian/rules get-orig-source.
> 
> Either one way is ok. After that, I'll sponsor your package.
> 
> BTW,
> I also have projects that needs tensorflow to be packaged into Debian.
> If you have any further tensorflow related packages you can directly
> mail me. Also I'm willing to help packaging those stuff.
> 
> Yours Sincerely,
> Paul
> 
> 


-- 
PaulLiu (劉穎駿)
E-mail: Ying-Chun Liu (PaulLiu) 



signature.asc
Description: OpenPGP digital signature

Bug#849273: RFS: gemmlowp/0~20170407-g719389f-1 [ITP]

[Ying-Chun Liu (PaulLiu)]

Hi Lumin,

I've reviewed your package. The package looks quite good to me.
The only thing missing is the README.source
https://www.debian.org/doc/debian-policy/ch-source.html#s-readmesource

The orig.tar.gz seems to be repacked from upstream git repo.
Please mention the steps on how you create the orig.tar.gz
Like you git clone it. And re-tar it, exclude some directories like .git.

Or, provide a way to generate the orig.tar.gz by debian/rules
get-orig-source

For example, added these to debian/rules
DEB_SOURCE := $(shell dpkg-parsechangelog | grep Source: | sed -e
's/Source: //'
)
DEB_VERSION := $(shell dpkg-parsechangelog | grep Version: | sed -e
's/Version: //')
DEB_UPSTREAM_VERSION := $(shell echo $(DEB_VERSION) | sed -e 's/-[^-]*$$//')
GIT_URL = https://github.com/google/gemmlowp.git

get-orig-source:
set -e; if echo $(DEB_VERSION) | grep -c "git"; \
then \
git_version=`echo $(DEB_VERSION) | sed -e
's/^.*~\([0-9]*\)*-\(.*\)-.*$$/\2/g'`; \
else \
git_version=$(DEB_UPSTREAM_VERSION); \
fi; \
tmpdir=`mktemp -d -t`; \
cd $$tmpdir; \
echo "checkout upstream repository ..."; \
git clone $(GIT_URL); echo "getting specific upstream
revision/tag: $$git_version"; \
cd `ls | head -n 1`; git checkout -b orig $$git_version; cd ..; \
tar --exclude=.git --exclude='*.exe' --exclude='*.jar' -cJvf
$(CURDIR)/$(DEB_SOURCE)_$(DEB_UPSTREAM_VERSION).orig.tar.xz `ls | head
-n 1`; \
cd $(CURDIR); \
rm -rf $$tmpdir

so when developer wants to pack the source tarball from the upstream,
they can use debian/rules get-orig-source.

Either one way is ok. After that, I'll sponsor your package.

BTW,
I also have projects that needs tensorflow to be packaged into Debian.
If you have any further tensorflow related packages you can directly
mail me. Also I'm willing to help packaging those stuff.

Yours Sincerely,
Paul


-- 
PaulLiu (劉穎駿)
E-mail: Ying-Chun Liu (PaulLiu) 



signature.asc
Description: OpenPGP digital signature