This is an automatically generated e-mail. To reply, visit: https://svn.reviewboard.kde.org/r/7111/

On May 19th, 2014, 2:33 p.m. UTC, Aurélien Gâteau wrote:

The change makes sense, but the code is quite complicated. I had a go at changing the way lupdate is used so that it creates correct file paths. This is what I came up with, does it work for you?


diff --git a/scripts/extract-tr-strings b/scripts/extract-tr-strings
index 3f65614..97ec12b 100755
--- a/scripts/extract-tr-strings
+++ b/scripts/extract-tr-strings
@@ -59,12 +59,15 @@ if [ ! -d "$enpodir" ] ; then
     die "The 'enpodir' environment variable does not point to an existing directory"
 fi
 
-# lupdate fails if the file passed as "-ts" argument already exists, so create a
-# temp dir where it can safely create a tmp.ts file.
-ts_dir=$(mktemp --directory --tmpdir ts-XXXXXX)
-trap "rm -rf $ts_dir" 0
+# lupdate fails if we pass it an existing but empty file, so we can't use a true
+# temporary file created with mktemp (we could create the file with mktemp and
+# rm it before passing it to lupdate, but that does not make it any nicer than
+# hardcoding the filename)
+# Note that $tmp_ts is created in the directory where Messages.sh is, to ensure
+# lupdate creates file paths relative to this directory.
+tmp_ts=extract-tr-strings-tmp.ts
+trap "rm -rf $tmp_ts" 0
 trap "exit 2" 1 2 3 13 15
-tmp_ts=$ts_dir/tmp.ts
 
 $LUPDATE -silent $src_files -ts $tmp_ts
 $LCONVERT $tmp_ts --sort-contexts --output-format pot -o $pot_file

On May 19th, 2014, 5:27 p.m. UTC, Alexander Potashev wrote:

Aurélien,

I was considering creating the temporary .ts file in the current directory too, but it is not the cleanest approach because
 1. You are modifying the source code tree (of course you can remove .ts file later),
 2. This won't work if you have no permission to write into the directory of the Git repo's working copy.

In my opinion the cleanest approach is to patch `lupdate` in qt-tools to add command line option forcing all source code paths to be relative to a specific directory (relative to $OLD_PWD, in our case).

On May 19th, 2014, 6:25 p.m. UTC, Albert Astals Cid wrote:

Well, then propose a patch to lupdate :D

On May 19th, 2014, 6:32 p.m. UTC, Alexander Potashev wrote:

I can make a patch for lupdate and it's going to be very simple, but then we'll need to wait for like a year until the patch is accepted (and if it is accepted) and then the new version of Qt is released. Or... can you run the manually patched lupdate on the l10n server?

On May 19th, 2014, 6:43 p.m. UTC, Albert Astals Cid wrote:

I won't diverge from upstream. We can patch something that has already been merged upstream but not yet released if needed, but carrying our own patches is not the way to go. If there's a bug, you fix it where the bug is :) Please submit your fix to http://codereview.qt-project.org/

On May 19th, 2014, 9:18 p.m. UTC, Aurélien Gâteau wrote:

I hesitated before creating the .ts file in the current directory, but then I remembered many projects already do create files from their Messages.sh file: they create a rc.cpp file when they use $EXTRACTRC. Therefore I think it's OK.

On May 19th, 2014, 10:03 p.m. UTC, Alexander Potashev wrote:

OK, now I agree with you.

Albert also says it's OK (in #kde-i18n at Freenode.net):
[01:37] <aspotashev_> tsdgeos: what do you think about using Aurelien's approach - create tmp.ts in the current directory when extract-tr-messages runs?
[01:38] <tsdgeos> aspotashev_: haven't checked to be honest
[01:39] <tsdgeos> aspotashev_: can you give me the link again
[01:39] <tsdgeos> can't find it :S
[01:45] <aspotashev_> tsdgeos: https://svn.reviewboard.kde.org/r/7111/
[01:46] <aspotashev_> see comments under Aurelien's review
[01:50] <tsdgeos> aspotashev_: it's ok, needs some code to make sure it's not overwriting that file in the 0.00000000000000001% case it exists
[01:50] <tsdgeos> and then it bails out and complains

On May 19th, 2014, 10:06 p.m. UTC, Alexander Potashev wrote:

Aurelien, can you please implement that check? I'm asking you because you already started the work on changing $tmp_ts.

Just filed https://svn.reviewboard.kde.org/r/7113/ which actually use a temp file in the source directory. If we want to ensure we are not overwriting an existing file, that sounds like the simplest and most resilient way to do it.

- Aurélien

On May 18th, 2014, 11:44 p.m. UTC, Alexander Potashev wrote:

Review request for Localization and Translation (l10n), Aurélien Gâteau and Albert Astals Cid.

By Alexander Potashev.

Updated May 18, 2014, 11:44 p.m.

Repository: kde

Description

Cleanup source code references in *_qt.po files generated with "extract-tr-strings"

Filenames in source code references should be represented as relative paths
with respect to the location of Messages.sh.

Testing

Ran ./Messages.sh in the source code repository for KAuth5 framework in its subdirectory "src". All directory elements upper than Messages.sh are stripped, including "/src/", e.g.:

-#: ../../home/scripty/prod/git-unstable/frameworks_kauth/src/backends/dbus/DBusHelperProxy.cpp:76
+#: backends/dbus/DBusHelperProxy.cpp:76
 msgctxt "KAuth::DBusHelperProxy|"
 msgid "DBus Backend error: connection to helper failed. "
 msgstr ""

Diffs

trunk/l10n-kf5/scripts/extract-tr-strings (1387292)
trunk/l10n-kf5/scripts/strip_ts_source_path_prefix.xsl (PRE-CREATION)

View Diff