Bug 176591

As this package can be built with different "main" phps, IMHO it is better to do
"phpize" (i.e., "phpize --clean" after untar to clear, and then "phpize" to
create needed stuff).
PHP-4.3.11 (FC3), PHP-5.0.4 (FC4) and PHP-5.1.1 (FC5) can have different phpize
results.

It seems that upstream developers already run some "phpize" (like an analogue of
autotools), but it is more robust to rerun it for different php versions.

Hint:

%prep
%setup -q -n php-json-ext-%{version}
phpize --clean
phpize

%build
%configure
make %{?_smp_mflags}

Updated.

OK

Remarks & nitpicks:
- autodetection of extdir and version can be simplified a little (as good
php-config always present in the Fedora's php-devel package)
- Use %{?dist} in the release field (as this package hardly depends to specific
distro's php version)
- Remove leading "An" from the summary
- Some %check section would be useful (at least to check that this module is
successfully attached at run-time and is visible by the /usr/bin/php).

See all these suggestions in the patch below.

I am confused a little by the fact there is no any documentation in this
package. Perhaps some data can be obtained from the upstream's site? (By simple
copying some *.html files etc.)

Created attachment 122613 [details]
suggested changes for the spec file

(In reply to comment #3)
> - autodetection of extdir and version can be simplified a little (as good
> php-config always present in the Fedora's php-devel package)

Except when mock does its depsolving.

> - Use %{?dist} in the release field (as this package hardly depends to
specific distro's php version)

I always add that once it's in CVS.

> - Remove leading "An" from the summary

Sorry, but I refuse to turn the Summary into grammatical garbage. Bring it up on
the mailing list if you wish to discuss this.

Updated.

> Except when mock does its depsolving.
Not understood how mock can have issues here, but it is not a blocker :)

> I always add that once it's in CVS.
Why not before? IMO the PackageGuidelines requires it at review stage... Anyway,
all other guys do it before CVS... 

- installed pam-json.html has execution bit set. Actually, "x" bit is set
immediately after "install -p %{SOURCE1}". Perhaps changing "install" to "cp"
can solve this.

- Initially, I resisted to increase release numbers after each trivial change.
But eventually various reviewers have forced me to do it always... ;)

(In reply to comment #6)
> > Except when mock does its depsolving.
> Not understood how mock can have issues here, but it is not a blocker :)

mock looks at the bare spec file and evaluates it, possibly before any BRs have
been installed. Try it some day.

> > I always add that once it's in CVS.
> Why not before? IMO the PackageGuidelines requires it at review stage... Anyway,
> all other guys do it before CVS... 

Most all other guys don't have %dist defined on their system, and it gives the
buildsystem conniptions if I import it with %dist as .fc4 and then try to tag in
FC-4.

> - installed pam-json.html has execution bit set. Actually, "x" bit is set
> immediately after "install -p %{SOURCE1}". Perhaps changing "install" to "cp"
> can solve this.

No, this is because I misused install. Fixed.

> - Initially, I resisted to increase release numbers after each trivial change.
> But eventually various reviewers have forced me to do it always... ;)

It's not supposed to be used by the public yet, so I figure no harm, no foul.

Updated.

> it gives the buildsystem conniptions if I import it with %dist as .fc4 and
then > try to tag in FC-4.
make tag TAG_OPTS="-F" ... :)

OK
APPROVED.

Built on FC4 and devel