2014-06-02 19:53:59

by Benno Schulenberg

[permalink] [raw]
Subject: [PATCH 1/4] po: describe more precisely the %B expansion

Signed-off-by: Benno Schulenberg <[email protected]>
---
po/at-expand.pl | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/po/at-expand.pl b/po/at-expand.pl
index d4427b5..bc1a744 100644
--- a/po/at-expand.pl
+++ b/po/at-expand.pl
@@ -21,10 +21,13 @@ my @translator_help = (
"#. on. A table of these expansions can be found below. Note that\n",
"#. %-expressions that begin with \"%D\" and \"%I\" are two-character\n",
"#. expansions; so for example, \"%Iu\" expands to the inode's user id\n",
- "#. ownership field (inode->i_uid).\n",
+ "#. ownership field (inode->i_uid). Also the \"%B\" expansion is special:\n",
+ "#. it can expand to either the string \"indirect block\" (possibly preceded\n",
+ "#. by the word \"double\" or \"triple\"), or the string \"block #\" immediately\n",
+ "#. followed by an integer indicating a block sequence number.\n",
"#. \n",
"#. %b <blk> block number\n",
- "#. %B <blkcount> integer\n",
+ "#. %B \"indirect block\" | \"block #\"<blkcount> string | string+integer\n",
"#. %c <blk2> block number\n",
"#. %Di <dirent> -> ino inode number\n",
"#. %Dn <dirent> -> name string\n",
--
1.7.0.4



2014-06-02 19:48:54

by Benno Schulenberg

[permalink] [raw]
Subject: [PATCH 2/4] e2fsck: make a prompt message translatable by not using fill-in words

Signed-off-by: Benno Schulenberg <[email protected]>
---
e2fsck/journal.c | 1 -
e2fsck/problem.c | 2 +-
2 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/e2fsck/journal.c b/e2fsck/journal.c
index a7b1150..a3b3967 100644
--- a/e2fsck/journal.c
+++ b/e2fsck/journal.c
@@ -820,7 +820,6 @@ errcode_t e2fsck_check_ext3_journal(e2fsck_t ctx)
no_has_journal:
if (!(sb->s_feature_compat & EXT3_FEATURE_COMPAT_HAS_JOURNAL)) {
recover = sb->s_feature_incompat & EXT3_FEATURE_INCOMPAT_RECOVER;
- pctx.str = "inode";
if (fix_problem(ctx, PR_0_JOURNAL_HAS_JOURNAL, &pctx)) {
if (recover &&
!fix_problem(ctx, PR_0_JOURNAL_RECOVER_SET, &pctx))
diff --git a/e2fsck/problem.c b/e2fsck/problem.c
index 2b564a8..e393417 100644
--- a/e2fsck/problem.c
+++ b/e2fsck/problem.c
@@ -225,7 +225,7 @@ static struct e2fsck_problem problem_table[] = {

/* Superblock has_journal flag is clear but has a journal */
{ PR_0_JOURNAL_HAS_JOURNAL,
- N_("@S has_@j flag is clear, but a @j %s is present.\n"),
+ N_("@S has_@j flag is clear, but a @j @i is present.\n"),
PROMPT_CLEAR, PR_PREEN_OK },

/* Superblock needs_recovery flag is set but not journal is present */
--
1.7.0.4


2014-06-02 19:48:59

by Benno Schulenberg

[permalink] [raw]
Subject: [PATCH 3/4] e2fsck: make two comments match the messages

Signed-off-by: Benno Schulenberg <[email protected]>
---
e2fsck/problem.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/e2fsck/problem.c b/e2fsck/problem.c
index e393417..3a00cd0 100644
--- a/e2fsck/problem.c
+++ b/e2fsck/problem.c
@@ -461,11 +461,11 @@ static struct e2fsck_problem problem_table[] = {
N_("Pass 1: Checking @is, @bs, and sizes\n"),
PROMPT_NONE, 0 },

- /* Root directory is not an inode */
+ /* Root inode is not a directory */
{ PR_1_ROOT_NO_DIR, N_("@r is not a @d. "),
PROMPT_CLEAR, 0 },

- /* Root directory has dtime set */
+ /* Root inode has dtime set */
{ PR_1_ROOT_DTIME,
N_("@r has dtime set (probably due to old mke2fs). "),
PROMPT_FIX, PR_PREEN_OK },
--
1.7.0.4


2014-06-02 19:53:57

by Benno Schulenberg

[permalink] [raw]
Subject: [PATCH 4/4] e2fsck: sort the abbreviations better

This makes it easier for translators to look up what they've done.

Signed-off-by: Benno Schulenberg <[email protected]>
---
e2fsck/message.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/e2fsck/message.c b/e2fsck/message.c
index 6c1d0bf..9c1433f 100644
--- a/e2fsck/message.c
+++ b/e2fsck/message.c
@@ -116,17 +116,17 @@ static const char *abbrevs[] = {
N_("Bbitmap"),
N_("ccompress"),
N_("Cconflicts with some other fs @b"),
- N_("iinode"),
- N_("Iillegal"),
- N_("jjournal"),
- N_("Ddeleted"),
N_("ddirectory"),
+ N_("Ddeleted"),
N_("eentry"),
N_("E@e '%Dn' in %p (%i)"),
N_("ffilesystem"),
N_("Ffor @i %i (%Q) is"),
N_("ggroup"),
N_("hHTREE @d @i"),
+ N_("iinode"),
+ N_("Iillegal"),
+ N_("jjournal"),
N_("llost+found"),
N_("Lis a link"),
N_("mmultiply-claimed"),
--
1.7.0.4


2014-06-02 23:40:03

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH 2/4] e2fsck: make a prompt message translatable by not using fill-in words

On Mon, Jun 02, 2014 at 09:48:42PM +0200, Benno Schulenberg wrote:
> Signed-off-by: Benno Schulenberg <[email protected]>
> ---
> e2fsck/journal.c | 1 -
> e2fsck/problem.c | 2 +-
> 2 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/e2fsck/journal.c b/e2fsck/journal.c
> index a7b1150..a3b3967 100644
> --- a/e2fsck/journal.c
> +++ b/e2fsck/journal.c
> @@ -820,7 +820,6 @@ errcode_t e2fsck_check_ext3_journal(e2fsck_t ctx)
> no_has_journal:
> if (!(sb->s_feature_compat & EXT3_FEATURE_COMPAT_HAS_JOURNAL)) {
> recover = sb->s_feature_incompat & EXT3_FEATURE_INCOMPAT_RECOVER;
> - pctx.str = "inode";
> if (fix_problem(ctx, PR_0_JOURNAL_HAS_JOURNAL, &pctx)) {
> if (recover &&
> !fix_problem(ctx, PR_0_JOURNAL_RECOVER_SET, &pctx))
> diff --git a/e2fsck/problem.c b/e2fsck/problem.c
> index 2b564a8..e393417 100644
> --- a/e2fsck/problem.c
> +++ b/e2fsck/problem.c
> @@ -225,7 +225,7 @@ static struct e2fsck_problem problem_table[] = {
>
> /* Superblock has_journal flag is clear but has a journal */
> { PR_0_JOURNAL_HAS_JOURNAL,
> - N_("@S has_@j flag is clear, but a @j %s is present.\n"),
> + N_("@S has_@j flag is clear, but a @j @i is present.\n"),

It's probably better to just make this read "but a journal is
present". The reason why we were doing the fill-in word was because
the intent was to be able to make the distinction between a journal
inode and an external journal device. The intent was never realied,
and at this point it's better to just take out the specifity of
whether we are using a journal inode or a journal device.

- Ted

2014-06-03 20:35:32

by Benno Schulenberg

[permalink] [raw]
Subject: [2/4 v2] e2fsck: make a prompt message simpler and thus translatable

It can be made simpler because there is no need to differentiate between
having an internal journal inode and having an external journal device.

Signed-off-by: Benno Schulenberg <[email protected]>

---
e2fsck/journal.c | 1 -
e2fsck/problem.c | 2 +-
2 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/e2fsck/journal.c b/e2fsck/journal.c
index a7b1150..a3b3967 100644
--- a/e2fsck/journal.c
+++ b/e2fsck/journal.c
@@ -820,7 +820,6 @@ errcode_t e2fsck_check_ext3_journal(e2fsck_t ctx)
no_has_journal:
if (!(sb->s_feature_compat & EXT3_FEATURE_COMPAT_HAS_JOURNAL)) {
recover = sb->s_feature_incompat & EXT3_FEATURE_INCOMPAT_RECOVER;
- pctx.str = "inode";
if (fix_problem(ctx, PR_0_JOURNAL_HAS_JOURNAL, &pctx)) {
if (recover &&
!fix_problem(ctx, PR_0_JOURNAL_RECOVER_SET, &pctx))
diff --git a/e2fsck/problem.c b/e2fsck/problem.c
index 2b564a8..e393417 100644
--- a/e2fsck/problem.c
+++ b/e2fsck/problem.c
@@ -225,7 +225,7 @@ static struct e2fsck_problem problem_table[] = {

/* Superblock has_journal flag is clear but has a journal */
{ PR_0_JOURNAL_HAS_JOURNAL,
- N_("@S has_@j flag is clear, but a @j %s is present.\n"),
+ N_("@S has_@j flag is clear, but a @j is present.\n"),
PROMPT_CLEAR, PR_PREEN_OK },

/* Superblock needs_recovery flag is set but not journal is present */

2014-06-04 02:05:45

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH 1/4] po: describe more precisely the %B expansion

On Mon, Jun 02, 2014 at 09:48:41PM +0200, Benno Schulenberg wrote:
> Signed-off-by: Benno Schulenberg <[email protected]>

Thanks, applied.

- Ted

2014-06-04 02:07:22

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [2/4 v2] e2fsck: make a prompt message simpler and thus translatable

On Tue, Jun 03, 2014 at 10:35:22PM +0200, Benno Schulenberg wrote:
> It can be made simpler because there is no need to differentiate between
> having an internal journal inode and having an external journal device.
>
> Signed-off-by: Benno Schulenberg <[email protected]>

You forgot to fix up the regression test suite f_extra_journal so
"make check" doesn't fail.

I've taken care of it, but in general before sending patches, I'd
appreicate a quick run of "make -j8 check" (or -j16 if you have a more
powerful machine).

Cheers,

- Ted

2014-06-04 02:07:36

by Theodore Ts'o

[permalink] [raw]

2014-06-04 02:07:48

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH 4/4] e2fsck: sort the abbreviations better

On Mon, Jun 02, 2014 at 09:48:44PM +0200, Benno Schulenberg wrote:
> This makes it easier for translators to look up what they've done.
>
> Signed-off-by: Benno Schulenberg <[email protected]>

Thanks, applied.

- Ted

2014-06-04 12:23:27

by Benno Schulenberg

[permalink] [raw]
Subject: Re: [2/4 v2] e2fsck: make a prompt message simpler and thus translatable


On Wed, Jun 4, 2014, at 4:07, Theodore Ts'o wrote:
> You forgot to fix up the regression test suite f_extra_journal so
> "make check" doesn't fail.
>
> I've taken care of it,

Thanks.

> but in general before sending patches, I'd
> appreicate a quick run of "make -j8 check"

Okay.

Running 'make check' now, I see these messages along the way:

mktemp: too few X's in template `r_64bit_big_expand.XXXXXX.tmp'
touch: missing file operand
Try `touch --help' for more information.
rm: missing operand
Try `rm --help' for more information.
r_64bit_big_expand: very large fs growth using ext4 w/64bit: skipped
mktemp: too few X's in template `r_bigalloc_big_expand.XXXXXX.tmp'
touch: missing file operand
Try `touch --help' for more information.
rm: missing operand
Try `rm --help' for more information.
r_bigalloc_big_expand: ext4 with bigalloc: skipped
mktemp: too few X's in template `r_ext4_big_expand.XXXXXX.tmp'
touch: missing file operand
Try `touch --help' for more information.
rm: missing operand
Try `rm --help' for more information.
r_ext4_big_expand: very large fs growth using ext4: skipped


That doesn't look intentional.
The contents of r_ext4_big_expand.log are:

very large fs growth using ext4 starting
dd: truncating at 2199023255552 bytes in output file `/tmp/e2fsprogs-tmp.I08Z35': File too large
using
dd: opening `': No such file or directory


On this system, 'info mktemp' says:
"The last six characters of this string must be `XXXXXX'."

So by doing a 's/XXXXXX.tmp/tmp.XXXXXX/' in tests/scripts/resize_test
those messages disappear and 144 tests succeeded 0 tests failed.

Patch is coming up in a minute.

Benno

--
http://www.fastmail.fm - IMAP accessible web-mail


2014-06-04 12:56:54

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [2/4 v2] e2fsck: make a prompt message simpler and thus translatable

On Wed, Jun 04, 2014 at 02:23:21PM +0200, Benno Schulenberg wrote:
>
> Running 'make check' now, I see these messages along the way:
>
> mktemp: too few X's in template `r_64bit_big_expand.XXXXXX.tmp'

Hmm... what OS and if Linux, version of coreutils are you using?

On my system, from "man mktemp":

Create a temporary file or directory, safely, and print its
name. TEMPLATE must contain at least 3 consecutive 'X's in
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
last component.
^^^^^^^^^^^^^^^

- Ted

2014-06-04 13:25:39

by Benno Schulenberg

[permalink] [raw]
Subject: Re: [2/4 v2] e2fsck: make a prompt message simpler and thus translatable


On Wed, Jun 4, 2014, at 14:56, Theodore Ts'o wrote:
> On Wed, Jun 04, 2014 at 02:23:21PM +0200, Benno Schulenberg wrote:
> >
> > Running 'make check' now, I see these messages along the way:
> >
> > mktemp: too few X's in template `r_64bit_big_expand.XXXXXX.tmp'

This "too few X's" is misleading -- what it intends to say is that it
sees zero X's in the final component of the name, "tmp".

> Hmm... what OS and if Linux, version of coreutils are you using?

Ubuntu Lucid, 2010.04, mktemp --version | head -1
mktemp (GNU coreutils) 8.13

> On my system, from "man mktemp":
>
> Create a temporary file or directory, safely, and print its
> name. TEMPLATE must contain at least 3 consecutive 'X's in
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> last component.
> ^^^^^^^^^^^^^^^

Same here. It still says "*last* component".

Regards,

Benno

--
http://www.fastmail.fm - The professional email service


2014-06-04 14:42:42

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [2/4 v2] e2fsck: make a prompt message simpler and thus translatable

On Wed, Jun 04, 2014 at 03:25:36PM +0200, Benno Schulenberg wrote:
>
> On Wed, Jun 4, 2014, at 14:56, Theodore Ts'o wrote:
> > On Wed, Jun 04, 2014 at 02:23:21PM +0200, Benno Schulenberg wrote:
> > >
> > > Running 'make check' now, I see these messages along the way:
> > >
> > > mktemp: too few X's in template `r_64bit_big_expand.XXXXXX.tmp'
>
> This "too few X's" is misleading -- what it intends to say is that it
> sees zero X's in the final component of the name, "tmp".

In the posix specification, "component" has a specific meaning, and it
means the last part of a pathname. For example the "basename" command
is described as removing all of the leading directory components.
NAME_LEN is defined to be the maximum length of a pathname component,
etc.

I don't think your reading of the word "component" is correct.

> > Hmm... what OS and if Linux, version of coreutils are you using?
>
> Ubuntu Lucid, 2010.04, mktemp --version | head -1
> mktemp (GNU coreutils) 8.13

OK, interesting. It seems to work just fine using coreutils 8.21
(Debian Testing). And I just checked on one of my Debian Stable
machines, which is using coreutils 8.13-3.5, and it's working there.

% mktemp /tmp/r_64bit_big_expand.XXXXXX.tmp
/tmp/r_64bit_big_expand.QAOy36.tmp

... and I just tried it on a Ubuntu 12.04 machine, and it accepts it
too:

8% cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=12.04
DISTRIB_CODENAME=precise
DISTRIB_DESCRIPTION="Ubuntu 12.04.4 LTS"
% mktemp -V
mktemp (GNU coreutils) 8.13
Copyright (C) 2011 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by Jim Meyering and Eric Blake.
% mktemp /tmp/r_64bit_big_expand.XXXXXX.tmp
/tmp/r_64bit_big_expand.6yNF5W.tmp


Hmm... I just checked on a Mac OSX machine, which is using a FreeBSD
userspace iirc, and its man page explicitly states that six X's must
be at the end of the pathname. And while "mktemp /tmp/foo.XXXXXX.bar"
doesn't fail, it returns "/tmp/foo.XXXXXX.bar", and if
/tmp/foo.XXXXXX.bar already exists, only then will it fail with a
"File exists" error message.

So it looks like allowing the placement of the XXX characters in the
middle of the filename is a GNU coreutils extension, and it must have
been introduced very shortly after Ubuntu 10.04 snapshotted its
coreutils package from Debian. And since most of our developers
aren't using something quite as antique as Ubuntu 10.04, and while we
do have someone doing test compiles on Mac OSX, the fact that mktemp
generally works there means that Andreas never saw any problems with
it.

- Ted

2014-06-04 15:38:13

by Benno Schulenberg

[permalink] [raw]
Subject: Re: [2/4 v2] e2fsck: make a prompt message simpler and thus translatable


On Wed, Jun 4, 2014, at 16:42, Theodore Ts'o wrote:
> So it looks like allowing the placement of the XXX characters in the
> middle of the filename is a GNU coreutils extension, and it must have
> been introduced very shortly after Ubuntu 10.04 snapshotted its
> coreutils package from Debian.

You are right. On Wed Nov 4 2009 there is this in coreutils' git:

-TEMPLATE must end in at least 3 consecutive `X's.\n\
+TEMPLATE must contain at least 3 consecutive `X's in last component.\n\

and:

-count_trailing_X_s (const char *s)
+count_consecutive_X_s (const char *s, size_t len)

So, you can probably drop the patch.

Benno

--
http://www.fastmail.fm - One of many happy users:
http://www.fastmail.fm/help/overview_quotes.html


2014-06-04 16:03:25

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [2/4 v2] e2fsck: make a prompt message simpler and thus translatable

On Wed, Jun 04, 2014 at 05:38:12PM +0200, Benno Schulenberg wrote:
> On Wed, Jun 4, 2014, at 16:42, Theodore Ts'o wrote:
> > So it looks like allowing the placement of the XXX characters in the
> > middle of the filename is a GNU coreutils extension, and it must have
> > been introduced very shortly after Ubuntu 10.04 snapshotted its
> > coreutils package from Debian.
>
> You are right. On Wed Nov 4 2009 there is this in coreutils' git:
>
> -TEMPLATE must end in at least 3 consecutive `X's.\n\
> +TEMPLATE must contain at least 3 consecutive `X's in last component.\n\
>
> and:
>
> -count_trailing_X_s (const char *s)
> +count_consecutive_X_s (const char *s, size_t len)
>
> So, you can probably drop the patch.

Well, I actually care about portability to *BSD and other legacy Unix
systems :-), so I plan to the change to push the XXXXXX to the end of
the template.

Thanks for pointing this out,

- Ted