cannot parse /etc/fstab with trailing "," in the fs_mntops field

[rwmjones]

$ mkdir -p root/etc
$ echo '/dev/mapper/foo-bar / xfs defaults, 0 0' > root/etc/fstab
$ cat root/etc/fstab
/dev/mapper/foo-bar / xfs defaults, 0 0

Note extra "," in the fourth field. Apparently the tools which normally parse fstab are fine with that.

$ augtool -r $PWD/root
augtool> get /augeas/files/etc/fstab/error/message
/augeas/files/etc/fstab/error/message = Iterated lens matched less than it should
augtool> get /augeas/files/etc/fstab/error/line
/augeas/files/etc/fstab/error/line = 1
augtool> get /augeas/files/etc/fstab/error/char
/augeas/files/etc/fstab/error/char = 34

Augeas fails to parse the file at the extra "," character.

[igalic]

can you provide a PR with a fix?
or at least one with a failing test?

[rwmjones]

I will try to at some point, although I'm pretty busy at the moment. The first comment does contain a test case which should be reproducible anywhere if you're just interested in seeing the failure.

[tupyy]

Hello,

I started to work on this one, but I've failed every time.
What I've done so far:
1 - tried to add a new sep after the options.

 let sep2 = del /,?[  \t]*/ " "

  let record = [ seq "mntent" .
                   Util.indent .
                   [ label "spec" . store spec ] . sep_tab .
                   [ label "file" . store file ] . sep_tab .
                   comma_sep_list "vfstype" .
                   (sep_tab . comma_sep_list "opt" .
                    (sep2 . [ label "dump" . store /[0-9]+/ ] .
                     ( sep_spc . [ label "passno" . store /[0-9]+/ ])? )? )?
                 . Util.comment_or_eol ]

This one failed with:

/home/cosmin/tmp/augeas/fstab.aug:27.2-35.40:Failed to compile record
/home/cosmin/tmp/augeas/fstab.aug:32.19-34.73:exception: ambiguous concatenation
      First regexp: /([ \t]+)(([^,#= \n\t]+)(((=)(([^,# \n\t]+)?))?))(((,)(([^,#= \n\t]+)(((=)(([^,# \n\t]+)?))?)))*)/
      Second regexp: /((,?[  \t]*)(([0-9]+))((([ \t]+)(([0-9]+)))?))?/
      ' A0' can be split into
      ' A|=|0'

     and
      ' A0|=|'

    First lens: /home/cosmin/tmp/augeas/fstab.aug:32.19-.50:
    Second lens: /home/cosmin/tmp/augeas/fstab.aug:33.20-34.73:

2: Add a new lens comma?

 let record = [ seq "mntent" .
                   Util.indent .
                   [ label "spec" . store spec ] . sep_tab .
                   [ label "file" . store file ] . sep_tab .
                   comma_sep_list "vfstype" .
                   (sep_tab . comma_sep_list "opt" . comma? .
                    (sep_tab . [ label "dump" . store /[0-9]+/ ] .
                     ( sep_spc . [ label "passno" . store /[0-9]+/ ])? )? )?
                 . Util.comment_or_eol ]

Failed with:

/`home/cosmin/tmp/augeas/fstab.aug:25.2-33.40:Failed to compile record
/home/cosmin/tmp/augeas/fstab.aug:30.53-.59:exception: optional expression matches the empty tree but does not consume a value

From the code, I understand that with this solution there is a problem in PUT direction but I don't know how to solve it.

Please advise on how I may tackle this one.

Thank you

[georgehansper]

Hi Cosmin,

I haven't had a chance to examine this at length, but can I suggest for (1) above use:

 let sep2 = del /,?[  \t]+/ " "

This is required because

/dev/mapper/foo-bar / xfs defaults,0 0

is not valid, and would match the original sep2 above

Regards,
George

[tupyy]

Hi @georgehansper

With your solution, augtool works fine:

➜  augeas cat etc/fstab

/dev/mapper/foo-bar / xfs default, 0 0
/dev/mapper/foo-bar / xfs not_with_comma 0 0

augeas augtool -I /home/cosmin/tmp/augeas

augtool> ls etc/fstab/
1/  2/

augtool> ls etc/fstab/1/
spec = /dev/mapper/foo-bar
file = /
vfstype = xfs
opt = default
dump = 0
passno = 0
augtool> ls etc/fstab/2/
spec = /dev/mapper/foo-bar
file = /
vfstype = xfs
opt = not_with_comma
dump = 0
passno = 0

augtool> set etc/fstab/1/opt new_value
augtool> ls etc/fstab/1/
spec = /dev/mapper/foo-bar
file = /
vfstype = xfs
opt = new_value
dump = 0
passno = 0

augtool> save

➜  augeas cat etc/fstab
/dev/mapper/foo-bar / xfs new_value, 0 0
/dev/mapper/foo-bar / xfs not_with_comma 0 0

but the tests are failing:

augparse -t -I /home/cosmin/tmp/augeas -I /usr/local/share/augeas/lenses/dist tests/test_fstab.aug
Module tests/test_fstab.aug loaded
Module /home/cosmin/tmp/augeas/fstab.aug loaded
Module /usr/local/share/augeas/lenses/dist/sep.aug loaded
Module /usr/local/share/augeas/lenses/dist/util.aug loaded
Module /usr/local/share/augeas/lenses/dist/rx.aug loaded
Module /usr/local/share/augeas/lenses/dist/build.aug loaded
tests/test_fstab.aug:160.2-168.3:exception thrown in test
tests/test_fstab.aug:160.7-.62:exception: Iterated lens matched less than it should
    Lens: /home/cosmin/tmp/augeas/fstab.aug:37.12-.42:
      Last match: /home/cosmin/tmp/augeas/fstab.aug:32.19-34.76:
      Not matching: /usr/local/share/augeas/lenses/dist/util.aug:111.22-.67:
    Error encountered at 1:39 (39 characters into string)
    </foo-bar / xfs defaults, 0 0|=|>

    Tree generated so far:


Syntax error in lens definition
Failed to load tests/test_fstab.aug

test_fstab.aug:

....(* Bug #832 *)
  test Fstab.lns get "/dev/mapper/foo-bar / xfs defaults, 0 0" =
  { "1"
    { "spec" = "/dev/mapper/foo-bar" }
    { "file" = "/" }
    { "vfstype" = "xfs" }
    { "opt" = "defaults" }
    { "dump" = "0" }
    { "passno" = "0" }
  }

(* Local Variables: *)
(* mode: caml       *)
(* End:             *)

[georgehansper]

Hi @tupyy

I can reproduce the error above, and I noticed that the error is at the end of the line.

It seem like the existing Fstab lens requires a newline at the end of each line, for the "syntax" to be "complete".

Adding the newline explicitly in the string like this:

  test Fstab.lns get "/dev/mapper/foo-bar / xfs defaults, 0 0\n" =

makes the error go away.

I notice that all the other unit tests in test_fstab.aug also have an explicit "\n" at the end of the string being tested

[tupyy]

@georgehansper nice catch. Thanks. I'll make the PR

[georgehansper]

Thanks for the PR

Can I ask for a couple of small things

1. the name sep_tab_comma would be more descriptive as sep_comma_tab
2. There is some trailing whitespace on line 158 of lenses/tests/test_fstab.aug the command "git diff HEAD~1" objects to this and highlights it in red, making the output less readable. Can you please remove the trailing spaces?

[tupyy]

@georgehansper done. Thank you for the review

[georgehansper]

Hi @tupyy

One last thing just occurred to me. The original lens used sep_tap aka Sep.tab in place of the (new) sep_comma_tab

In the interests of avoiding unnecessary changes, we should make sep_comma_tab generate a tab char, instead of a space.
ie.

let sep_comma_tab = del /,?[ \t]+/ "\t"

Can I ask you to make this change to PR #838 ?

[tupyy]

@georgehansper done.