Überprüfen auf Sonderzeichen im Dateinamen

[jskeletti]

Ich habe bei unserer Subversion Installation einen "pre-commit-hook" geschrieben, der die zu übertragenen Dateien auf Sonderzeichen (<>:"/\|?*) überprüfen soll. Das klappt soweit. Leider bekomme ich Probleme, sobald irgendwelche Sonderzeichen wie Umlaute (äöüß) oder Sonderzeichen aus anderen Sprachen (hier polnisch: ąłż etc) im Dateinamen sind. Das liegt darin, dass Python diese anscheinend als folgenden String interpretiert: "t?\\195?\\164st.txt" (hier "täst.txt"). Wie kann ich mein Script dazu bewegen, nur die richtigen Sonderzeichen herauszufiltern? Dazu würde es ja theoretisch reichen, dass der String mit dem Dateinamen die Umlaute nicht ins "?\\195?\\164" übersetzt.

[lunar]

Python „interpretiert“ da nichts. Das ist entweder eine Eigenart von Subversion, oder – wahrscheinlicher – ein Fehler in Deinem Quelltext. Ich nehme an, Du dekodierst die Dateinamen nicht richtig, und verwendest intern kein Unicode, sondern arbeitest stumpf auf den Bytes der Namen.

Möchtest Du detaillierte Hilfe, musst Du uns schon den Quelltext zeigen. Wir können schließlich nicht hellsehen…

[jskeletti]

Die wichtigsten Stellen im Quellcode:

Code: Alles auswählen

...
retVal = 0
chars = set('<>:"/\|?*')

####################
## Helper methods ##
####################

# Gets a command's output
def commandOutput(command):
    import subprocess
    process = subprocess.Popen(command.split(), stdout = subprocess.PIPE)
    return process.communicate()[0] # 0 is stdout

# Returns an array of the changed files' names
def getChangedFiles(svnPath, transaction):
    # Run svnlook to find the files that were changed
    output = commandOutput('%s changed %s --transaction %s' % (SVNLOOK, svnPath, transaction))

    # The output from svnlook looks like the following:
    # U   folder/file1.cpp
    # A   folder/file2.cpp
    # where U means updated and A means added
    def changed(fileName):
        return len(line) > 0 and line[0] in ('A', 'U')
    changedFiles = [line[4:] for line in output.split('\n') if changed(line)]

    # svnlook inserts an empty line, so output.split() will have an extra
    # line with nothing in it - ignore the last lines if they're empty
    if changedFiles:
      while 0 == len(changedFiles[-1]):
        changedFiles = changedFiles[:-1]

      return changedFiles
    else:
      return []

...

###################################################
### Verify filenames if compatible with windows ###
###################################################
files = getChangedFiles(repopath, transact)

for file in files:
  file =  os.path.basename(file)
  print file + "\n"
  print f + "\n"
  print g + "\n"

  if any((c in chars) for c in file):
    sys.stderr.write("Filename (%r) not compatible with Windows!\n" % (file))
    retVal = 1

if retVal == 1:
    sys.stderr.write("Please check the characters in the filename(s)!\n")
    sys.exit(retVal)

...

[Sirius3]

@jskeletti: das hängt mit dem Encoding zusammen, was »svn« zur Ausgabe der Dateinamen benutzt. Wenn es Zeichen nicht ausgeben kann, werden diese eben mit "?\xxx" codiert.
Das ist also kein Problem mit Python.

Code: Alles auswählen

import subprocess

# Returns an array of the changed files' names
def get_changed_files(svnPath, transaction):
    # Run svnlook to find the files that were changed
    process = subprocess.Popen([SVNLOOK, "changed", svnPath, "--transaction", transaction], stdout = subprocess.PIPE)
    output = process.communicate()[0].splitlines() # 0 is stdout
    return [line.split(None,1)[-1] for line in output if line.startswith('A') or line.startswith('U')]

[jskeletti]

OK; aber lässt es sich mit Python umschiffen oder lösen? Oder kann man an der Konfiguration vom SVN etwas drehen?

[Sirius3]

Du mußt halt in Deinen Environment-Variablen irgendeine »locale« setzen, die UTF-8 codiert, z.b. 'de_DE.UTF-8'.

[BlackJack]

@jskeletti: Du könntest Dir eine Funktion zum dekodieren der Zeichenketten nach Unicode schreiben, oder zumindest die ``?\123``-Sequenzen das (Byte-)Zeichen mit dem entsprechenden Wert umwandeln. Das ginge mit dem `re`-Modul recht einfach. Bei `re.sub()` kann man als Ersetzung auch eine Funktion angeben die das `Match`-Objekt übergeben bekommt und die Ersetzung zurück geben muss.

Sonstige Anmerkungen zum Quelltext: Der hält sich in so einigem nicht an den Style Guide for Python Code. Insbesondere mit der Einrücktiefe sollte man nicht experimentieren, damit erschwert man die Zusammenarbeit mit anderen nur unnötig.

Auf Modulebene sollte man nur Konstanten, Funktionen, und Klassen definieren und keinen Programmcode schreiben. Denn dann kann man ein Modul nicht importieren ohne dass der Code abläuft, zum Beispiel um einzelne Funktionen zu manuell oder automatisiert zu testen, oder wieder zu verwenden. Wenn der Code in Funktionen steckt, besteht auch nicht Gefahr, dass man aus versehen (oder absichtlich) auf Datenstrukturen zugreift, die eine Funktion nicht als Argument betreten haben, und man sich so undurchsichtige Abhängigkeiten bastelt. Noch unübersichtlicher wird es wenn man Programmlogik auf Moduleben mit der Definition von Funktionen vermischt, weil man sich den Programmablauf dann zwischen den Funktionsdefinitionen zusammen suchen muss.

Der Kommentar ``Helper Methods`` ist technisch gesehen falsch, weil es Funktionen sind. Bringt der überhaupt einen Mehrwert? Gibt es denn in dem Programm noch andere Funktionen die nicht unter diese Überschrift passen?

Die Kommentare über den beiden gezeigten Funktionen wären als DocStrings besser platziert.

``import``-Anweisungen sollten am Anfang des Moduls stehen und nicht innerhalb von Funktionen, solange man dafür keinen guten Grund hat.

So wie Du mit dem `command` verfährst handelst Du Dir Probleme ein wenn der Pfad zum Beispiel Leerzeichen oder andere „whitespace”-Zeichen enthält.

Die `changed()`-Funktion bekommt ein Argument übergeben, verwendet das dann aber gar nicht.

Die ``while``-Schleife zum entfernen der Leerzeilen am Ende ist ineffizient, weil da in jedem Durchlauf die gesamte Liste minus des letzten Elements kopiert wird. Ein entfernen des letzten Elements mittels `pop()` wäre effizienter. Aber der ganze Code nach dem Filtern ist sowieso unnötig weil Leerzeilen doch schon an dem Test mit der `change()`-Funktion gescheitert und rausgeflogen wären.

`file` ist der Name eines eingebauten Typs, den sollte man nicht an ein anderes Objekt binden. Zumal es sich gar nicht um Dateien sondern Datei*namen* handelt.

Bist Du sicher dass Deine Lösung auch verbotene Zeichen in Verzeichnisnamen findet?

Das Ganze könnte dann ungefähr so aussehen:

Code: Alles auswählen

#!/usr/bin/env python
import os
import re
import sys
from subprocess import PIPE, Popen

SVNLOOK = 'svnlook'
SVN_ESCPAPE_RE = re.compile(r'\?\\(\d{1,3})')


def decode_svn_filename(path):
    SVN_ESCPAPE_RE.sub(path, lambda m: chr(int(m.group(1))))


def execute_command(command):
    """Gets a command's output."""
    process = Popen(command, stdout=PIPE)
    output, _error_output = process.communicate()
    return output


def iter_changed_paths(repo_path, transaction):
    """Returns an array of the changed files' names"""
    # 
    # Run svnlook to find the files that were changed.
    # 
    output = execute_command(
        [SVNLOOK, 'changed', repo_path, '--transaction', transaction]
    )
    # 
    # The output from svnlook looks like the following:
    # U   folder/file1.cpp
    # A   folder/file2.cpp
    # where U means updated and A means added.
    # 
    return (
        line.split(None, 1)[-1]
        for line in output.splitlines()
        if line.startswith(('A', 'U'))
    )


def main():
    result = 0
    forbidden_chars = set('<>:"/\|?*')

    # ...
    
    repo_path = '/'
    transaction = 42  # XXX: Just an example, not a real value.
    #
    # Verify filenames if compatible with windows.
    #
    filenames = (
        decode_svn_filename(os.path.basename(path))
        for path in iter_changed_paths(repo_path, transaction)
    )
    for filename in filenames:
        print filename + '\n'
        if any((c in forbidden_chars) for c in filename):
            sys.stderr.write(
                'Filename (%r) not compatible with Windows!\n' % filename
            )
            result = 1

    if result == 1:
        sys.stderr.write('Please check the characters in the filename(s)!\n')
        sys.exit(result)


if __name__ == '__main__':
    main()

[jskeletti]

Danke für die ausführliche Antwort. wenn man das so liest könnte man denken, dass ich ja gar nix richtig gemacht habe . Aber Ja, es funktioniert, für die Sonderzeichen, leider nicht für äöü etc. Ich probiere mein Script an Deine Vorschläge anzupassen.

[BlackJack]

@jskeletti: Zumindest wenn Du nur die Zeichen aus dem `set` entfernen möchtest, dann machen Umlaute doch keine Probleme. Die Dateinamen sind nach dem Ersetzen von den ``?\123``-Sequenzen immer noch UTF-8 kodiert, aber Du willst ja nur ASCII-Zeichen entfernen. Da besteht also keine Gefahr. Wenn Du auch die Umlaute als einzelne Zeichen haben möchtest, müsstest Du noch ein ``decode('utf-8')`` anwenden.

[jskeletti]

Da scheint noch etwas nicht zu klappen. An der Stelle

Code: Alles auswählen

def decode_svn_filename(path):
    SVN_ESCAPE_RE.sub(path, lambda m: chr(int(m.group(1))))

Bekomme ich folgenden Fehler:

TypeError: expected string or buffer

Hast Du vielleicht einen Tipp?

Nachtrag:
Wenn ich dort ein

Code: Alles auswählen

sys.stderr.write('--> '+path+'\n')

eingebe, bekomme ich als Ausgabe:

--> t?\195?\164st.txt

(Sollte eigentlich täst.txt sein)

[jskeletti]

Bist Du sicher dass Deine Lösung auch verbotene Zeichen in Verzeichnisnamen findet?

Verzeichnisnamen sind hier egal, da sich SVN darum kümmert! Ich muss nur die Dateinamen überprüfen. Das war in meiner Lösung gewährleistet.

[BlackJack]

@jskeletti: `sub()` erwartet die Argumente in umgekehrter Reihenfolge. Sorry.

[jskeletti]

Also

Code: Alles auswählen

SVN_ESCPAPE_RE.sub(lambda m: chr(int(m.group(1))), path)

So bekomme ich zwar keinen Fehler mehr, aber es wird nicht auf Sonderzeichen geachtet. filename ist None.

if any((c in forbidden_chars) for c in filename):
TypeError: 'NoneType' object is not iterable

[BlackJack]

@jskeletti: Da fehlt noch ein ``return``, sonst gibt die Funktion natürlich `None` zurück. Nochmal sorry…

[jskeletti]

Ohhh man, da hätte ich auch allein drauf kommen dürfen... Vielen herzlichen Dank. Klappt wunderbar!!! Kann man den Thread auf gelöst setzen?