public inbox for cygwin-apps@cygwin.com
 help / color / mirror / Atom feed
From: Yaakov Selkowitz <yselkowi@redhat.com>
To: cygwin-apps@cygwin.com
Subject: Re: Request for review: anthy 9100h+0.4 (Re: Update of packages by non-maintainer)
Date: Wed, 03 Jun 2020 12:41:14 -0400	[thread overview]
Message-ID: <7a01c2da88317fbbbdcf5f6f4a4b783fad32117b.camel@redhat.com> (raw)
In-Reply-To: <20200604.012050.1431284441844398224.yasu@utahime.org>

On Thu, 2020-06-04 at 01:20 +0900, Yasuhiro KIMURA wrote:
> From: Yasuhiro KIMURA <yasu@utahime.org>
> > So I gave up to use override.hint and decided to change version number
> > to "9100h+0.4". It works fine with all of cygport, mksetupini and
> > setup-x86_64.
> 
> Anyway version number issue has sloved. So I would like to request for
> review of anthy 9100h+0.4.

Not ready yet; see comments inline.

> * As for cygwin local patch, 9100h-no-undefined.patch is changed to
>   make it fit to new source tree. Others are removed.
[snip]
> * Add "MAKEOPTS=-j1" because parallel make causes build error.

That's the whole purpose of the exeext patch, to fix parallel make. 
Before you remove patches, you first need to understand what the patch
does, why it was needed, and see if it still is or not.  If you don't
know, then you should be asking rather than just dropping them.

> PKG_NAMES="${NAME} lib${NAME}0 lib${NAME}-common lib${NAME}-devel emacs-${NAME}"
[snip]
> libanthy0_REQUIRES="libanthy-common"
> libanthy0_CONTENTS="usr/bin/cyganthy*.dll"

This is wrong; most of the DLLs will be -1.dll, so this needs to be
libanthy1 (but see below!!).

> 9100h-no-undefined.patch:

This looks mostly fine, except:

> --- origsrc/anthy-0.4/src-util/Makefile.am      2019-07-05 11:37:13.000000000 +0900
> +++ src/anthy-0.4/src-util/Makefile.am  2020-05-23 00:59:26.155191100 +0900
> @@ -26,5 +26,6 @@
>  libanthyinput_la_SOURCES = input.c rkconv.c rkhelper.c\
>   rkconv.h rkmap.h rkhelper.h
>  libanthyinput_la_LIBADD = ../src-main/libanthy.la
> +libanthyinput_la_LDFLAGS = -no-undefined
>  
>  pkgdata_DATA = typetab dic-tool-usage.txt

This should probably have -version-info flag as well upstream,
otherwise you'll have conflicts with libanthy0 and libanthy1.  

-- 
Yaakov Selkowitz
Senior Software Engineer - Platform Enablement
Red Hat, Inc.



  reply	other threads:[~2020-06-03 16:41 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200521.134708.978300984572490822.yasu@utahime.org>
     [not found] ` <e40a75dd-5d73-19cd-2eeb-0625bb29d9f8@gmail.com>
     [not found]   ` <e777c90f-40f9-2276-1e6c-f8e5ee67678e@gmail.com>
2020-05-28 22:34     ` Update of packages by non-maintainer Yasuhiro KIMURA
2020-05-29  9:59       ` Achim Gratz
2020-06-03  5:31         ` Yasuhiro KIMURA
2020-06-03 16:20           ` Request for review: anthy 9100h+0.4 (Re: Update of packages by non-maintainer) Yasuhiro KIMURA
2020-06-03 16:41             ` Yaakov Selkowitz [this message]
2020-06-04 17:04               ` Request for review: anthy 9100h+0.4 Yasuhiro KIMURA

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7a01c2da88317fbbbdcf5f6f4a4b783fad32117b.camel@redhat.com \
    --to=yselkowi@redhat.com \
    --cc=cygwin-apps@cygwin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).