Bug 193154 - Review Request: asymptote - Descriptive vector graphics language
Summary: Review Request: asymptote - Descriptive vector graphics language
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Gérard Milmeister
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-05-25 18:05 UTC by Jose Pedro Oliveira
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-06-01 18:46:39 UTC
Type: ---
Embargoed:


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

Description Jose Pedro Oliveira 2006-05-25 18:05:26 UTC
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 22:48:47 UTC
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 15:27:39 UTC
(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 15:35:31 UTC
(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 16:36:10 UTC
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 17:48:50 UTC
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 17:58:08 UTC
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 18:03:46 UTC
(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 18:11:20 UTC
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 19:43:52 UTC
(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 20:00:22 UTC
(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-27 01:09:03 UTC
> 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 10:53:00 UTC
* 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 13:54:17 UTC
(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 14:27:13 UTC
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 16:52:36 UTC
(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 18:46:39 UTC
Thanks for the review.

Imported and built for FC-4, FC-5, and devel.

Comment 17 Jose Pedro Oliveira 2006-06-01 19:02:36 UTC
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.