Bugzilla Bug 101727

Converts .fig to OpenOffice-Draw files. Can be used by sci-chemistry/chemtool
if
installed.

Created an attachment (id=67509) [edit]
ebuild for fig2sxd

I wrote an ebuild for fig2sxd and it works fine for me on x86. 
Any feedback is welcome.

Markus:    
    
I was searching for something like this some time ago. ;-) Thanks for the    
ebuild. Here are a few things that could be improved:    
    
- We usually try to simplify "SRC_URI" as much as we can by using ${PN},     
  ${PV}, and ${P}. You already use ${PV}, but you could use ${PN} too.  
- There are other places (3) where you could use ${PN} instead of fig2sxd.  
  (The only place where these variable substitutions should be avoided is in   
  "HOMEPAGE"; it is easier to open the link in a browser.)  
- When there are two or more files in "SRC_URI", each should be put one line    
  for improved readability.    
- There is a special "mirror://" syntax that can be used in "SRC_URI" for     
  networks such as the Gentoo or sourceforge mirrors. Have a look at other     
  ebuilds that pull files from sourceforge.    
- "DEPEND" and "RDEPEND" need not be declared since they are empty. (But it is    
  correct (mandatory in fact) to declare "IUSE" even if it is empty.)    
- All variable assignments must be quoted. This is especially important when    
  variable substitutions are used inside an assignment (check on line 23).    
- All variable substitutions that are related to customisable paths must be    
  quoted. For instance, 'cd "${S}"' rather than 'cd ${S}'. The latter would    
  fail if there were special characters or spaces in "PORTDIR". ${WORKDIR}    
  should also be quoted.  
- You should always check the return status of commands that modify files   
  using the "die" function. For instance:  
      sed -e "s/foo/bar/g" -i Makefile || die  
  (Note that epatch checks for the return status automatically so "die" is not   
  needed.)  
- The above example will also allow you to use sed without temporary files,   
  which is a bit more elegant.  
- You should also use "die" when installing files manually. For instance:  
      doman ${PN}.1 || die  
  (The "do" series of function do not check the return code and will not die  
  automatically like epatch. This is because some eclasses use them to install  
  basic documentation (README, etc.) that may be missing in some packages, and  
  it looks like we were too lazy to implement proper checks in those   
  cases. ;-) The bottom line is you must use "|| die" anytime a failure would  
  result in the package not being properly installed.)  
- The ebuild does not honor the user's C++ compiler. If you type     
  'CXX="icc"

Markus:    
    
I was searching for something like this some time ago. ;-) Thanks for the    
ebuild. Here are a few things that could be improved:    
    
- We usually try to simplify "SRC_URI" as much as we can by using ${PN},     
  ${PV}, and ${P}. You already use ${PV}, but you could use ${PN} too.  
- There are other places (3) where you could use ${PN} instead of fig2sxd.  
  (The only place where these variable substitutions should be avoided is in   
  "HOMEPAGE"; it is easier to open the link in a browser.)  
- When there are two or more files in "SRC_URI", each should be put one line    
  for improved readability.    
- There is a special "mirror://" syntax that can be used in "SRC_URI" for     
  networks such as the Gentoo or sourceforge mirrors. Have a look at other     
  ebuilds that pull files from sourceforge.    
- "DEPEND" and "RDEPEND" need not be declared since they are empty. (But it is    
  correct (mandatory in fact) to declare "IUSE" even if it is empty.)    
- All variable assignments must be quoted. This is especially important when    
  variable substitutions are used inside an assignment (check on line 23).    
- All variable substitutions that are related to customisable paths must be    
  quoted. For instance, 'cd "${S}"' rather than 'cd ${S}'. The latter would    
  fail if there were special characters or spaces in "PORTDIR". ${WORKDIR}    
  should also be quoted.  
- You should always check the return status of commands that modify files   
  using the "die" function. For instance:  
      sed -e "s/foo/bar/g" -i Makefile || die  
  (Note that epatch checks for the return status automatically so "die" is not   
  needed.)  
- The above example will also allow you to use sed without temporary files,   
  which is a bit more elegant.  
- You should also use "die" when installing files manually. For instance:  
      doman ${PN}.1 || die  
  (The "do" series of function do not check the return code and will not die  
  automatically like epatch. This is because some eclasses use them to install  
  basic documentation (README, etc.) that may be missing in some packages, and  
  it looks like we were too lazy to implement proper checks in those   
  cases. ;-) The bottom line is you must use "|| die" anytime a failure would  
  result in the package not being properly installed.)  
- The ebuild does not honor the user's C++ compiler. If you type     
  'CXX="icc" emerge fig2sxd', g++ is used and not icc. Have a look at the    
  toolchain-funcs eclass. It has a tc-getCXX function that you could use    
  to solve this problem. "sci-biology/phylip" is an example of a package where    
  the user's C compiler is honored using toolchain-funcs.    
    
All in all, these are minor problems and the ebuild is already rather good.    
I'll add it to Portage as soon as you provide me with a fixed version. 
   
In answer to your questions, I agree that 0.13 is the correct version to use   
here. Using sed (or other basic UNIX tools) is correct as long as it does not  
happen in global space, since that would slow down Portage (the tool would be  
run everytime the ebuild is sourced) and might break things because we cannot  
assume the dependency on a tool is satisfied when the ebuild is sourced.  
Inside a function, it is perfectly OK.  

Created an attachment (id=67570) [edit]
improved ebuild for fig2sxd

Hi Olivier,

Thanks a bunch for the great suggestions; I hope I implemented them
all correctly, if not please let me know. 

One question comes to mind: I don't have icc installed on my box, hence
even though the ebuild now honors the C++ compiler I can not confirm that
it actually compiles. Should one enforce the use of gcc for a particular ebuild

until it is confirmed that other compilers work or steps have been taken 
in the ebuild depending on the type of compiler? 

Added to portage as media-gfx/fig2sxd with little changes (you don't need sed
to
override variables in a Makefile and I wanted to have the changelog installed).
Thanks Markus and Olivier!