LinuxQuestions.org

LinuxQuestions.org (/questions/)
-   Slackware (http://www.linuxquestions.org/questions/slackware-14/)
-   -   Bug in makepkg and symlinks with blanks in filename? (http://www.linuxquestions.org/questions/slackware-14/bug-in-makepkg-and-symlinks-with-blanks-in-filename-4175480597/)

DarkVision 10-13-2013 05:49 AM

Bug in makepkg and symlinks with blanks in filename?
 
Hi...
i was trying to speed up packaging of the kawoken-icon-theme which includes hundreds of symlinks. makepkg takes ages here to move the links to the doinst.sh script. I was playing with the find-command to create a doinst.sh contain the same symlinks that would be created later by much slower code used by makepkg.

When i checked that my find-command would produce the same doinst.sh code like makepkg i figured out that some links by makepkg were wrong if the symlink or filename includes blanks. And there are a few in this package.

Here is a small test script that creates a testing package with only one file and a symlink, both with blanks in the filename.

The output of my find command uses " around filenames so blanks will not split filenames any longer. The output of makepkg is different and in my opinion not correct:
Code:

#!/bin/sh
TMPDIR=test-$(mcookie)
mkdir -p ${TMPDIR}/usr/bin
touch "${TMPDIR}/usr/bin/This is a filename with blanks"
ln -sf "This is a filename with blanks" "${TMPDIR}/usr/bin/This is a symlink with blanks"

echo "Make symlinks in doinst.sh using find..."
# Add this to find to remove the symlinks: -exec rm -rf "{}" \;
( cd ${TMPDIR}
  find usr/ -type l -printf "( cd \"%h\" ; rm -rf \"%f\" )\n( cd \"%h\" ; ln -sf \"%l\" \"%f\" )\n"
)

echo "Make symlinks in doinst.sh using makepkg..."
( cd ${TMPDIR}
  makepkg -l y -c n ../test-1.0-noarch.tgz &>/dev/null
)
cat ${TMPDIR}/install/doinst.sh

And here is the output:
Quote:

Make symlinks in doinst.sh using find...
( cd "usr/bin" ; rm -rf "This is a symlink with blanks" )
( cd "usr/bin" ; ln -sf "This is a filename with blanks" "This is a symlink with blanks" )
Make symlinks in doinst.sh using makepkg...
( cd usr/bin ; rm -rf This )
( cd usr/bin ; ln -sf a This )
Was this ever discussed here? Should makepkg take care about filenames with blanks or should the source tarball be fixed by the original maintainer or package maintainer?

I don't take care about it since my find-code seem to work with those special symlinks (at least a diff against makepkgs doinst.sh for this special case does not show much differences except filenames with blanks) and speeds up packing from about 50min down to 3min but i wonder if this is something that should be fixed or not...

tronayne 10-13-2013 06:18 AM

You know, it might be better to "fix" the problem before you start.

Here's a little shell program, blank_rename.sh, that I use prior to doing anything with "sentence" file names; it replaces the spaces with underscores, file names, directory names, whatever.

Kinda handy when you get stuff from winidiots...

Hope this helps some.
Code:

#!/bin/bash
#ident        "$Id$"
#
#        This program is free software; you can redistribute it and/or
#        modify it under the terms of version 2 of the GNU General
#        Public License as published by the Free Software Foundation.
#
#        This program is distributed in the hope that it will be useful,
#        but WITHOUT ANY WARRANTY; without even the implied warranty of
#        MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
#        General Public License for more details.
#
#        You should have received a copy of the GNU General Public
#        License along with this program; if not, write to the Free
#        Software Foundation, Inc., 59 Temple Place - Suite 330, Boston,
#        MA 02111-1307, USA.
#
#        Name:                $Source$
#        Version:        $Revision$
#        Modified:        $Date$
#        Purpose:        Replace blanks in file names with underscores
#        Author:                Unknown
#        Date:                28 Feb 2010
#        $Log$

ONE=1                                # For getting singular/plural right (see below).
number=0                        # Keeps track of how many files actually renamed.
FOUND=0                                # Successful return value.

for filename in *                # Traverse all files in directory.
do
    echo "$filename" | grep -q " "        #  Check whether filename
    if [ $? -eq $FOUND ]                  #+ contains space(s).
    then
      fname=$filename                      # Yes, this filename needs work.
      n=`echo $fname | sed -e "s/ /_/g"`  # Substitute underscore for blank.
      mv "$fname" "$n"                    # Do the actual renaming.
      let "number += 1"
    fi
done 

if [ "${number}" -eq "$ONE" ]                # For correct grammar.
then
        echo "${number} file renamed."
else
        echo "${number} files renamed."
fi

exit 0


DarkVision 10-13-2013 06:24 AM

I know how to handle filenames with blanks, this is not the problem. Thanks anyway for your script.

I also don't use blanks in filenames myself and i know why.

But can you be sure that every source package available does not make use of blanks in filenames? If you can not be sure... you should either take care about it in your program or at least show a warning that the source files include some stupid filenames.

As i said... i don't take care about it. I already added a sanity check to my scripts just in case there are other source packages with that kind of filenames.

P.S. And i'm not sure if icon names with blanks replaced by underscore will continue to work.

tronayne 10-13-2013 06:58 AM

I can't recall ever seeing any Unix/Linux package that had spaces (or carets or vertical bars or dollar signs (or euros or pounds) anything meaningful to the shell) in file or directory names -- only crap from Windows. Anytime I do get anything at all from Windows users I just walk the tree and change all the file masks (644 for files, 755 for directories) and use that little shell program to fix the spaces. I do that first, saves a heckuva lot of time and trouble. If there happens to be a Makefile with embedded spaces (and quoted file names), I can deal with that with an editor quick and and easy but, on the other hand, I've never tried to build anything from Windows so I dunno what kind of a mess it would be.

I seem to remember a CD-ROM that came with a book on graphics that was all Windows source. And, of course, it was also all Windows-specific graphic libraries. Tossed the book and used the CD-ROM for scaring away birds from the garden (they're really pretty when the sun is shining and there's a little breeze).

In Unix/Linux systems, a file name is just a file name and if you have a graphic it won't matter if there's an underscore in it, it'll open just like any other file would; there's almost never anything embedded that would prevent that, at least nothing I know of.

Sorry that it didn't help.

DarkVision 10-13-2013 07:10 AM

Well... the package i was trying to build is an icon theme especially for linux KDE or XFCE so it is nothing windows specific.

The filename that su**z is
Quote:

Google Docs-docs.google.com.png -> gnome-documents.png
While using google i found another theme that uses filenames like that:
moka-icon-theme

I'm sure this is a really rare condition and maybe there are no other source packages out there using such stupid filenames... but who knows?

P.S. Here is another filename (there is no symlink for it so makepkg would not fail here... anyway):
Quote:

Yahoo Mail-mail.yahoo.com.png
And i'm sure you cannot rename icon names without patching the applications that are looking for such icon names.

rkfb 10-13-2013 08:20 AM

I think I agree with DarkVision.

If you write a program that accepts input and someone throws input at it that it can't handle then the sensible solution must be to adjust your program accordingly. Even if, as in this scenario, the program exits and says 'Please fix the spaces in the filenames'.

The better solution would probably be rewrite the program code so it can deal with the spaces.

You may write a program and think you've covered all eventualities but once you've released it in to the wild you never know what users will do with it.

DarkVision 10-13-2013 08:37 AM

Quote:

Originally Posted by rkfb (Post 5044871)
Even if, as in this scenario, the program exits and says 'Please fix the spaces in the filenames'.

Indeed better then nothing but that would not fix applications that depend on filenames you "fixed" for the current package like for these icon themes...

I will rebuild a bunch of packages and check for those stupid filenames but i'm sure these are the only ones causing trouble here.

gnashley 10-14-2013 12:37 AM

Such things are rare in 'nix-ish sources, but do occur. I recently had the problem (spaces in filenames)when building and packaging MIT 'scratch'. I definitely agree that it should be fixed in makepkg -if you simply hope to notice the problem when it occurs and fix the problem in the sources, you'll more likely miss the problem.

Alien Bob 10-14-2013 05:47 AM

I can not speak for Patrick, but I think that suggestions for fixes in Slackware's native tools will be taken more seriously when they are accompanied by patches that actually fix the issue.

Eric

volkerdi 10-14-2013 03:10 PM

I did spend some time looking at this last night. What I'll note is that a solution isn't going to be as easy as wrapping arguments in quotes in all cases. It would be fine to use quotes *only* if spaces are detected in a filename or directory, but otherwise the script lines need to be produced so that they are exactly the same as the ones made by the existing script. Otherwise, comparisons between scripts made with the new script and scripts made with the old script aren't going to work.

DarkVision 10-14-2013 11:26 PM

Quote:

Originally Posted by volkerdi (Post 5045639)
It would be fine to use quotes *only* if spaces are detected in a filename or directory, but otherwise the script lines need to be produced so that they are exactly the same as the ones made by the existing script.

I'm not good at that but maybe this one gives the real good programmers an idea how to fix it. At least the output in doinst.sh seem to do what you have mentioned... only if spaces are detected the names are wrapped in quotes. Sample output:
Quote:

( cd usr/bin ; rm -rf Another_Filename )
( cd usr/bin ; ln -sf "Another Symlink" Another_Filename )
( cd usr/bin ; rm -rf "This is a symlink with blanks" )
( cd usr/bin ; ln -sf "This is a filename with blanks" "This is a symlink with blanks" )
And this is what i modified in the makepkg code. Since i'm not good with 'cut' i'm using sed:
Code:

make_install_script() {
  COUNT=1
  LINE="$(sed -n "$COUNT p" $1)"
  while [ ! "$LINE" = "" ]; do
  #LINKGOESIN="$(echo "$LINE" | cut -f 1 -d " ")"
  LINKGOESIN="$(echo "$LINE" | sed "s, -> .*,,g")"
  # dirname needs quotes for LINKGOESIN!
  LINKGOESIN="$(dirname "$LINKGOESIN")"
  #LINKNAMEIS="$(echo "$LINE" | cut -f 1 -d ' ')"
  LINKNAMEIS="$(echo "$LINE" | sed "s, -> .*,,g")"
  LINKNAMEIS="$(basename "$LINKNAMEIS")"
  #LINKPOINTSTO="$(echo "$LINE" | cut -f 3 -d ' ')"
  LINKPOINTSTO="$(echo "$LINE" | sed "s,.* -> ,,g")"
  # *** New lines... ***
  if [ x"${LINKGOESIN}" != x"${LINKGOESIN// /}" ]; then
    LINKGOESIN="\"${LINKGOESIN}\""
  fi
  if [ x"${LINKNAMEIS}" != x"${LINKNAMEIS// /}" ]; then
    LINKNAMEIS="\"${LINKNAMEIS}\""
  fi
  if [ x"${LINKPOINTSTO}" != x"${LINKPOINTSTO// /}" ]; then
    LINKPOINTSTO="\"${LINKPOINTSTO}\""
  fi
  # *** End of new lines... ***
  echo "( cd $LINKGOESIN ; rm -rf $LINKNAMEIS )"
  echo "( cd $LINKGOESIN ; ln -sf $LINKPOINTSTO $LINKNAMEIS )"
  COUNT=$(expr $COUNT + 1)
  LINE="$(sed -n "$COUNT p" $1)"
  done
}

The input for make_install_script is correct and gives the correct filenames. So i tried to split the filename from the linkname using sed-command. A comparison between original string and string with removed spaces will add quotes to the string.

As i said.. i'm not good at that, and i'm sure someone else has a much better idea, but might that be a way to handle it?

ruario 10-15-2013 04:23 AM

How about something like this.

Note: I have only very quickly tested this as I am at work and I don't really have the time to do this right now. It worked for a single, test package. I also have no doubt it will need more work but perhaps it is a initial step towards something better and maybe it will give someone ideas to play around with:

makepkg.patch:
Code:

--- makepkg.orig
+++ makepkg
@@ -195,11 +195,11 @@
 # Get rid of possible pre-existing trouble:
 INST=$(mktemp $TMP/makepkg.XXXXXX)
 # This requires the ls from coreutils-5.0 (or newer):
-find . -type l -exec ls -l --time-style=long-iso {} \; | while read foo ; do echo $foo ; done | cut -f 8- -d ' ' | cut -b3- | tee $INST
+find . -type l -printf '%p@@ARROW@@%l\n' | sed 's,^./,,;s/ /@@SPACE@@/g;s/@@ARROW@@/ -> /' | tee $INST | sed 's/@@SPACE@@/\\ /g'
 if [ ! "$(cat $INST)" = "" ]; then
  echo
  echo "Making symbolic link creation script:"
-  make_install_script $INST | tee doinst.sh
+  make_install_script $INST | sed 's/@@SPACE@@/\\ /g' | tee doinst.sh
 fi
 echo
 if [ ! "$(cat $INST)" = "" ]; then

removepkg.patch (Note: There are simpler example patches to removepkg in one of my later comments):
Code:

--- removepkg.orig
+++ removepkg
@@ -149,7 +149,9 @@
 }
 
 extract_links() {
- sed -n 's,^( *cd \([^ ;][^ ;]*\) *; *rm -rf \([^ )][^ )]*\) *) *$,\1/\2,p'
+ sed 's/\\ /@@SPACE@@/g' |\
+ sed -n 's,^( *cd \([^ ;][^ ;]*\) *; *rm -rf \([^ )][^ )]*\) *) *$,\1/\2,p' |\
+ sed 's/@@SPACE@@/\\ /g'
 }
 
 preserve_file() {
@@ -228,7 +230,7 @@
  if [ -L "$ROOT/$LINK" ]; then
    if [ ! "$WARN" = "true" ]; then
    echo "  --> Deleting symlink $ROOT/$LINK"
-    rm -f $ROOT/$LINK
+    rm -f "$ROOT/$LINK"
    else
    echo "  --> $ROOT/$LINK (symlink) would be deleted"
    fi

P.S. The really scary part part of doing stuff like this is it is easy to break stuff in subtle ways and risking breaking the install/remove functionality of Slackware is obviously a very bad idea. I'd personally be tempted to fix the very few packages that have spaces locally (and report a bugs upstream) rather than make big changes to Pkgtools. It just seems a lot safer!

DarkVision 10-15-2013 12:20 PM

Quote:

Originally Posted by ruario (Post 5045941)
P.S. The really scary part part of doing stuff like this is it is easy to break stuff in subtle ways and risking breaking the install/remove functionality of Slackware is obviously a very bad idea. I'd personally be tempted to fix the very few packages that have spaces locally (and report a bugs upstream) rather than make big changes to Pkgtools. It just seems a lot safer!

I agree on you with that part that it might be a risk and maybe breaks existing behavior... but think what happens if the package maintainer isn't aware of this problem: There can be commands in doinst.sh that may overwrite existing files because of the corrupt filenames in the doinst.sh script. Think of this example:
Quote:

Make symlinks in doinst.sh using find...
( cd "sbin" ; rm -rf "makepkg is a nice tool" )
( cd "sbin" ; ln -sf "makepkg-helper-tool" "makepkg is a nice tool" )
Make symlinks in doinst.sh using makepkg...
( cd sbin ; rm -rf makepkg )
( cd sbin ; ln -sf a makepkg )
OK, a bad example but depending on the first word of the symlink existing files may get killed...

For the kawoken-icon-theme you may get some 8000 links in the doinst.sh ... might be difficult to find files / symlinks with spaces in there just by reading the script created by makepkg.

Of course you could check before creating the package if there are symlinks or files with spaces. BTW i just checked against /var/log/packages here and there are a few packages that have filenames with spaces (python 2.7 for example, found a few custom packages also). This does not mean that there is a generic problem with spaces, i have not tested what happens if i remove a package that only contains filenames and directories with spaces but no symlinks. I just want to make clear that there are packages around that are using spaces in filenames and it might be also more complex to work around that problem by patching the applications. For the kawoken-icon-theme you may need to patch applications that use those icon-names too, not only the icon-theme itself.

And you are correct that the other tools like removepkg also need to be fixed, i hadn't that in mind.

If this is not going to be fixed there should be at least a warning in makepkg that there might be filenames/symlinks with spaces. A simple find command could help here.

hpfeil 10-15-2013 01:25 PM

The code wrapped in parentheses executes in an asynchronous sub-shell. I'm not sure it that sub-shell inherits the parent environment. I'm confused by the printf statement. Its parens probably need to be escaped with a backslash. I don't see an -exec command in the find statement.

ruario 10-15-2013 02:33 PM

@hpfeil: I'm not sure what you are talking about. If you are talking about DarkVision's suggested fix and use of subshells, have a look at your original makepkg, it uses subshells to generate the same variables.

As for my use of find's builtin printf, I'm not sure what you think is wrong but it generates the correct string. It does not need an exec because I do not execute an external command. Nonetheless, I have just tweaked it slightly to make it more readable.

EDIT: If you feel I changed that find command too much here is an alternative version, that just tweaks the original very slightly. I warn you however, it ain't pretty!

Code:

find . -type l -exec ls -l --time-style=long-iso {} \; | while read foo ; do echo $foo ; done | cut -f 8- -d ' ' | cut -b3- | sed 's/ -> /@@ARROW@@/;s/ /@@SPACE@@/g;s/@@ARROW@@/ -> /' | tee $INST | sed 's/@@SPACE@@/\\ /g'


All times are GMT -5. The time now is 11:47 AM.