Bug 193154 - Review Request: asymptote - Descriptive vector graphics language
Review Request: asymptote - Descriptive vector graphics language
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Gérard Milmeister
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-05-25 14:05 EDT by Jose Pedro Oliveira
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-06-01 14:46:39 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
example spec file (1.75 KB, application/octet-stream)
2006-05-26 13:48 EDT, Gérard Milmeister
no flags Details
asy-mode autoload file for emacs (116 bytes, text/x-emacs-lisp)
2006-05-26 13:58 EDT, Gérard Milmeister
no flags Details

  None (edit)
Description Jose Pedro Oliveira 2006-05-25 14:05:26 EDT
Spec URL:
http://gsd.di.uminho.pt/jpo/software/fedora/asymptote.spec

SRPM URL:
http://gsd.di.uminho.pt/jpo/software/fedora/asymptote-1.06-1.src.rpm

Description:
Asymptote is a powerful descriptive vector graphics language for technical
drawings, inspired by MetaPost but with an improved C++-like syntax.
Asymptote provides for figures the same high-quality level of typesetting
that LaTeX does for scientific text.
Comment 1 Gérard Milmeister 2006-05-25 18:48:47 EDT
Here a few things from first sight:
* Why do you copy things into the docdir instead of just using %doc?
  Also the doc directory is not owned.
  A proposal for what I did instead of the patch:
  add the option docdir=doc-install to configure, then you
  can simply use %doc doc-install/* in the files section:
    %doc LICENSE README TODO BUGS
    %doc doc-install/*
    %doc doc/asymptote.pdf
* The package must own directory %{_datadir}/%{name}
* The package must own directory %{texpkgdir}
* Possibly add a file asy-init.el to /usr/share/emacs/site-lisp/site-start.d:
  (autoload 'asy-mode "asy-mode.el" "Asymptote major mode." t)
  (add-to-list 'auto-mode-alist '("\\.asy$" . asy-mode))
  and something analogous for xemacs.

Output of rpmlint:
W: asymptote symlink-should-be-relative /usr/share/emacs/site-lisp/asy-mode.el
/usr/share/asymptote/asy-mode.el
W: asymptote symlink-should-be-relative /usr/share/vim/vim64/syntax/asy.vim
/usr/share/asymptote/asy.vim
W: asymptote symlink-should-be-relative /usr/share/vim/vim70/syntax/asy.vim
/usr/share/asymptote/asy.vim
W: asymptote symlink-should-be-relative /usr/share/vim/vim63/syntax/asy.vim
/usr/share/asymptote/asy.vim
W: asymptote symlink-should-be-relative
/usr/share/xemacs/site-packages/lisp/asy-mode.el /usr/share/asymptote/asy-mode.el
W: asymptote dangerous-command-in-%trigger ln
W: asymptote dangerous-command-in-%trigger ln
W: asymptote dangerous-command-in-%trigger rm
W: asymptote dangerous-command-in-%trigger rm
W: asymptote percent-in-%trigger
W: asymptote dangerous-command-in-%trigger ln
W: asymptote percent-in-%trigger
W: asymptote dangerous-command-in-%trigger rm
W: asymptote percent-in-%trigger
W: asymptote dangerous-command-in-%trigger rm
Comment 2 Jose Pedro Oliveira 2006-05-26 11:27:39 EDT
(In reply to comment #1)
>   A proposal for what I did instead of the patch:
>   add the option docdir=doc-install to configure, then you
>   can simply use %doc doc-install/* in the files section:
>     %doc LICENSE README TODO BUGS
>     %doc doc-install/*
>     %doc doc/asymptote.pdf

Don't you have two different directories with doc files
( /usr/share/doc/asymptote-1.0x/  and  /doc-install/) ?

Can you send the configure patch to me? And I will try to have it applied
uptream. In the past two months I have been able to have some
patches/suggestions applied upstream (one of them was the DESTDIR variable).

> * Possibly add a file asy-init.el to /usr/share/emacs/site-lisp/site-start.d:
>   (autoload 'asy-mode "asy-mode.el" "Asymptote major mode." t)
>   (add-to-list 'auto-mode-alist '("\\.asy$" . asy-mode))
>   and something analogous for xemacs.

I'm not a Emacs/Xemacs user but I rather have it upstream. Can you attach
a full asy-mode.el to this ticket? I will send it upstream.

jpo
Comment 3 Jose Pedro Oliveira 2006-05-26 11:35:31 EDT
(In reply to comment #1)
> Output of rpmlint:
> W: asymptote symlink-should-be-relative /usr/share/emacs/site-lisp/asy-mode.el
> /usr/share/asymptote/asy-mode.el
> W: asymptote symlink-should-be-relative /usr/share/vim/vim64/syntax/asy.vim
> /usr/share/asymptote/asy.vim
> W: asymptote symlink-should-be-relative /usr/share/vim/vim70/syntax/asy.vim
> /usr/share/asymptote/asy.vim
> W: asymptote symlink-should-be-relative /usr/share/vim/vim63/syntax/asy.vim
> /usr/share/asymptote/asy.vim
> W: asymptote symlink-should-be-relative
> /usr/share/xemacs/site-packages/lisp/asy-mode.el /usr/share/asymptote/asy-mode.el
> W: asymptote dangerous-command-in-%trigger ln
> W: asymptote dangerous-command-in-%trigger ln
> W: asymptote dangerous-command-in-%trigger rm
> W: asymptote dangerous-command-in-%trigger rm
> W: asymptote percent-in-%trigger
> W: asymptote dangerous-command-in-%trigger ln
> W: asymptote percent-in-%trigger
> W: asymptote dangerous-command-in-%trigger rm
> W: asymptote percent-in-%trigger
> W: asymptote dangerous-command-in-%trigger rm

All these trigger scripts are very fragile. The emacs/xemacs triggers
are almost identically to ones shipped in the fedora-rpmdevtools package.
The vim triggers are a little more complicated as vim includes the major
version in its directories (I had to ghost a lot of directories - vim63 for
FC-4, vim64 for FC-5, and vim70 for devel - in order to avoid unowned directories).

 
Comment 4 Jose Pedro Oliveira 2006-05-26 12:36:10 EDT
Package update:
http://gsd.di.uminho.pt/jpo/software/fedora/asymptote.spec
http://gsd.di.uminho.pt/jpo/software/fedora/asymptote-1.06-2.src.rpm

Changelog:
Corrects the directories ownership.
Comment 5 Gérard Milmeister 2006-05-26 13:48:50 EDT
Created attachment 130037 [details]
example spec file

Here is the spec file I used. The doc-install is a temporary directory.
Thus I can better decide what to include and not rely on "make install".
Comment 6 Gérard Milmeister 2006-05-26 13:58:08 EDT
Created attachment 130038 [details]
asy-mode autoload file for emacs

This must go to:
for emacs:
/usr/share/emacs/site-lisp/site-start.d/asy-init.el
for xemacs:
/usr/share/xemacs/site-packages/lisp/site-start.d/asy-init.el
Comment 7 Jose Pedro Oliveira 2006-05-26 14:03:46 EDT
(In reply to comment #5)
> Created an attachment (id=130037) [edit]
> example spec file
> 
> Here is the spec file I used. The doc-install is a temporary directory.
> Thus I can better decide what to include and not rely on "make install".

That isn't enough as the docdir location is stored in a header file by the
configure script.  Right now I need to patch the configure script.

jpo
Comment 8 Gérard Milmeister 2006-05-26 14:11:20 EDT
Hmm, I don't see where the problem. Maybe there is a misunderstanding...
The files specified using %doc are copied to %{_docdir}/%{name}-%{version}
and this directory with its content automatically included in the rpm.
Comment 9 Jose Pedro Oliveira 2006-05-26 15:43:52 EDT
(In reply to comment #8)
> Hmm, I don't see where the problem. Maybe there is a misunderstanding...

Please correct me if I misunderstood the following lines from comment #1

<quote> 
  A proposal for what I did instead of the patch:
  add the option docdir=doc-install to configure, then you
  can simply use %doc doc-install/* in the files section:
</quote>

I read the above as:

 1) please drop your patch because I have a better solution

 2) you mention an configure option that I thought you had added (patch?)
    Now that I have seen your specfile you don't even touch the configure
    script or even override the configure macro default directories.

Do you want me to drop the patch or not?

> The files specified using %doc are copied to %{_docdir}/%{name}-%{version}
> and this directory with its content automatically included in the rpm.

Yes I know.

I also think I finally understood what you meant by overriding the docdir
makefile variable (not configure as stated in comment #1).  But I don't like
the solution as it appears to be more error prone (for example the main pdf
file - asymptote.pdf - fails to be installed in the correct directory ant has to
be manually added to the doc list).

jpo
Comment 10 Gérard Milmeister 2006-05-26 16:00:22 EDT
(In reply to comment #9)
>  1) please drop your patch because I have a better solution
No, it was just a proposal, so there would be no need for the
patch to configure. Patching configure can be annoying, since
often a new patch must be created for a new version.

>  2) you mention an configure option that I thought you had added (patch?)
>     Now that I have seen your specfile you don't even touch the configure
>     script or even override the configure macro default directories.
> 
> Do you want me to drop the patch or not?
Do whatever you prefer.

You may package the asy-init.el file, these init files are normally
not provided by upstream.
Comment 11 Jose Pedro Oliveira 2006-05-26 21:09:03 EDT
> You may package the asy-init.el file, these init files are normally
> not provided by upstream.

Done.

http://gsd.di.uminho.pt/jpo/software/fedora/asymptote.spec
http://gsd.di.uminho.pt/jpo/software/fedora/asymptote-1.06-3.src.rpm
Comment 12 Gérard Milmeister 2006-05-27 06:53:00 EDT
* package naming: ok
* license good and included: ok
* spec file layout: ok
* source md5: ok
* builds in mock/i386: ok
* buildreq: ok
* no locales
* owns all directories it creates: ok
* correct use of $RPM_BUILD_ROOT: ok
* documentation in the right place: ok
* no header files or shared libraries
* no desktop file
* works as expected on examples: ok
* triggers generate warnings by rpmlint: ignore

? group tag is Development/Tools
  I suggest Applications/Publishing in analog to tetex (and metapost)

APPROVED
  
Comment 13 Jose Pedro Oliveira 2006-05-27 09:54:17 EDT
(In reply to comment #12)
> ...
> ? group tag is Development/Tools
>   I suggest Applications/Publishing in analog to tetex (and metapost)

Done.

> APPROVED

Thanks for the review.

Do you mind if I send the emacs init file upstream?

jpo
Comment 14 Jose Pedro Oliveira 2006-05-29 10:27:13 EDT
The texinfo package has been splitted in Fedora Core 6.

   FC-5(texinfo) = FC-6(texinfo, texinfo-tex)

Changed the build requirement
    BuildRequires:  texinfo
to
    BuildRequires:  /usr/bin/texi2dvi

jpo
Comment 15 Gérard Milmeister 2006-05-29 12:52:36 EDT
(In reply to comment #13)
> Do you mind if I send the emacs init file upstream?
No Problem.
Comment 16 Jose Pedro Oliveira 2006-06-01 14:46:39 EDT
Thanks for the review.

Imported and built for FC-4, FC-5, and devel.
Comment 17 Jose Pedro Oliveira 2006-06-01 15:02:36 EDT
Random/pending notes:

Upstream will release version 1.07 in the next days:
 * includes the emacs init file
 * make install-all will also install the info file (patches are now all in svn)
   new BR: ImageMagick

The build requirement
  /usr/bin/texi2dvi
can now be replaced by
  texinfo-tex
As of today's updates, the FC-4 and FC-5 texinfo RPM started also providing
texinfo-tex.

Due to performance reasons upstream would also like to see the package compiled
 * with -O3, and
 * with a single threaded version of gc
  (single-threaded instead multi-threaded --> 15% improvement)


Note You need to log in before you can comment on or make changes to this bug.