Segmentation fault when parsing pkgdeps with wrong type of comments: '#' i.o. '//'

[jim-bcom]

HOW TO REPRODUCE

• create a packagedependencies.txt file containing only

# This comment does not use the right syntax and causes a crash

• run remaken install on it

EXPECTED BEHAVIOR

• A parsing error of some sort

OBSERVED BEHAVIOR

Segmentation fault

This seems to occur in the Dependency ctor, while calling boost::split method.

[stefled]

Hello

which remaken version is used?

with remaken 1.10.0 pre-release, observed behaviour is :

[IGNORED]: Dependency line '# This comment does not use the right syntax and causes a crash' with invalid format !

[jim-bcom]

Hi,

I've used remaken 1.10.0, but it was an older release apparently.

I've tested with the latest ( https://github.com/b-com-software-basis/remaken/releases/tag/1.10.0%2Funixes ), and I can reproduce your output.

But does it work as intended ?

• Isn't there supposed to be a syntax for comments, e.g. a line starting with // ?
  Then, these comments should be ignored, and by ignored, I mean maybe there shouldn't be these [IGNORED] message for every comments in the file (ig there was a license header for exemple there'd be multiple useless lines printed for each remaken command invokation.

• In my version, [IGNORED] is prompted for a valid comment (line starting with //), but other lines seems to be treated as ill-formed dependencies, and crash. Maybe it's due to this fix of the latest 1.10.0, but now, it's not that lines starting with # are now treated as comments, it's that some forms of ill-formed deps are ignored, and other forms are not and raise an error. For example with this file:

// Comment 1
# wrong comment syntax
/* another wrong comment syntax */
Not commented at all
MyDep|1.0.0|foo
MyDep/1.2.0|MyDep|MyDep@github|https://URL|shared

Results in the following output after remaken install

[IGNORED]: Dependency line '// Comment 1' is commented !
[IGNORED]: Dependency line '# wrong comment syntax' with invalid format !
[IGNORED]: Dependency line '/* another wrong comment syntax */' with invalid format !
[IGNORED]: Dependency line 'Not commented at all' with invalid format !
[IGNORED]: Dependency line 'MyDep|1.0.0|foo' with invalid format !
[2024-02-27 09:25:37.084864] [0x00007fb2ad6ba740] [error]   Dependency file error : invalid repository type : unsupported value= https://URL
Dependency =>
    name=MyDep@github
    version=MyDep
    type=https://URL
    link mode=shared
    identifier=https://URL
    channel=stable

[2024-02-27 09:25:37.084973] [0x00007fb2ad6ba740] [error]   Error parsing dependency file : invalid format

To summarize:

• proper comments (// currently I think, although I find # more natural for this kind of file, maybe being used to Bash) should be silently ignored, as any comment is in other tools. Or at the very least, only prompted in verbose mode, but even then, I'm not sure.
• Any other uncommented lines should be considered as a dependency description, and thus a error should be raised when ill-formed (as it is the case for the last example.

What do you think ?

Also, I've tried to add a comment at the end of a well-formed dep, but it seems to be interpreted as a mode:

[WARNING]: Current mode 'shared // plop' is unsupported for 'MyDeps' dependency, then apply 'shared'default mode

[jim-bcom]

Sorry, I just noticed that comments and ill-formed deps are indeed treated diffferently in the last version

[IGNORED]: Dependency line '// Comment 1' is commented !
[IGNORED]: Dependency line '# wrong comment syntax' with invalid format !

So, for these two case, to me, the first line shouldn't be printed (or at least in verbose mode), and second line et alike maybe shouldn't have [IGNORED] printed.

What do you think ?

[stefled]

The current way to comment a dependency is to start with // and this is actually displayed to the developer.
It is possible to put it only in verbose mode, but that seems dangerous/useless to me.

For me comments or an ignored lign in a packagedependencies.txt file should only be used for local testing purposes, and I find it useful to systematically trace whether a dependency is in comments.

In the case of a ill-formatted dependency which starts with # or other, I chose to ignore the dependency and not to stop the execution with an error.
I can change to raise an error, but the ignore message text is explicit

[stefled]

in order to display less messages to users, the [IGNORED] dependencies comments are only displayed in verbose mode

wil be available in next 1.11.0 version

[jim-bcom]

Ok for having something less verbose, but having the choice of seeing ignored deps in verbose mode.
But correct me if I'm wrong, in this last version, every comments is considered as a commented deps and will appear as [IGNORED]: Dependency line ?
This means, pkgdep format does not allow "real" comments, for documention?
Do you think it could be valuable to a have that? Not being able to comment is sometimes not handy (c.f. JSON).
For example there could one comment syntax for deps and one for comments.

# Ignore this deps because of issue #123
// MyDeps|1.2.3|...

$ remaken install -v
[IGNORED]: MyDeps|1.2.3| ...

[stefled]

Actually, a comment for documentation or a commented dependency is the same thing and must begin by '//'
I don't think we must add a new behaviour for distinguish the two things

on the other hand, we could note :

• [COMMENT] for comments,
  and
• [IGNORED] for badly formatted dependencies

[jim-bcom]

Well, to me, we should not print "real" comments, so either we never print what's commented (the usual/expected way of dealing with comments) if we want the same behavior in all cases, or discriminate between comments and deps if we want to print ignored deps.

I'm not sure I understood you proposal, but you mean replacing an error for badly formatted by ignoring them? I think if a dep is badly formatted, we should have an error to correct it, no?

That's my take, but you have the last word.