Erroneous targets are returned by the parser

[Leffmann]

The following makefile,

A = vasmm68k_mot -quiet -spaces -nosym -I$(VBCC)/library -I$(NDK39I)
C = vc -c99 -O1 -sc -sd -lvcs

default:    test test.library
clean:
        del test test.library

test:       test.c
        $(C) -o test test.c

test.library:   test.library.s
        $(A) -Fhunkexe -o test.library test.library.s

should reasonably only yield the four obvious targets, but instead returns the following nine targets, default (no args), default, clean, make, makefile, test, test.library, test.c, test.library.s.

[lslezak]

It seems that the new functionality for reading the Makefile targets does not work properly in some cases. I have tested it at the https://github.com/openSUSE/snapper repository which uses the GNU autotools and found these issues:

• At first it did not report any targets besides the default one. After some debugging I found that there was a stdout maxBuffer exceeded exception silently ignored (at https://github.com/AtomBuild/atom-build-make/blob/master/lib/make.js#L41). The error should be probably reported via atom.notifications.addError or at least logged to the console.
• I run make -prRn manually at the console to see the real output and it revealed that it is huge, it had about 86.000 lines and roughly 4.3 MB(!) in size. Increasing the buffer size by using the maxBuffer: 5*1024*1024 option in the exec call helped, but still...
• Some targets were displayed multiple times (like all) or some found targets did not work at all. It turned out that they were reported for the recursive Makefiles located in the subdirectories. These obviously do not work for the top level Makefile.

Maybe the Makefile should be simply parsed without running make at all? (Like the competitive build-tools-make do?)

[lslezak]

One more issue:

• It reported invalid targets like if test ! -f config.h; then rm -f stamp-h1; else :; fi which by chance match the expected regexp...

[Leffmann]

I agree, it seems a better idea to just parse the makefile itself. I would suggest a simpler regex such as /^[a-z0-9_-]+:/gim, because most of the time you're just interested in the "top level" targets, but don't care about the ones that build swaths of smaller objects.

Another suggestion would be to scan the makefile for a simple tag that tells atom-build-make what targets you're interested in, f.ex. #atom-build-make: app libs clean, and ignoring everything else. This way I also have the option to ignore the default target if I'd like to.

It would also be a good idea to present the targets in the order they appear in the makefile, this way you can format and place the interesting targets at the top, and not have to scroll through the list to find them.

[noseglid]

Thanks for the valid input on this new feature.

One of the main reasons I wanted to use make to retrieve the targets was to be able to cope with complex situations where makefiles are included and other difficult-to-generalize situations.

I think maybe a configuration option is in place, which will use a simpler regex (as proposed by @leffmann) instead of the more complicated solution.

[noseglid]

I pushed a branch which has this option and only regexes the Makefile for targets. It's available on branch target-extraction-choice if anyone wants to give it a whirl

[Leffmann]

I can't make it recognize any targets in the makefile I posted earlier, it now only adds its own default.

[noseglid]

@leffmann Did you test #9?

[Leffmann]

Yes I did. I get the new option to use make for target extraction, and when unchecked it drops from 9 erroneous targets to just the default (no args) target.

Does it work on your side? I admit to being a beginner with Git, and I may have messed up when putting this repo into packages/build-make. Starting with a clean install of 0.6.0, how would you go about doing this?

[noseglid]

It works for me with the makefile you provided:

Essentially:

• Uninstall build-make and remove any references you have (e.g. in ~/.atom/package)
• git clone git@github.com:AtomBuild/atom-build-make.git
• cd atom-build-make
• apm install
• apm link
• Restart Atom

(I haven't tried the step above, but if anything errors you can probably figure it out :smile: )

[Leffmann]

I installed a fresh Atom, and everything works exactly like it should on Mac, but breaks on Windows. What a surprise.

I can do more tests if you'd like, otherwise I'll let you close when you want. Thanks for the help, Alexander!

[noseglid]

That's weird, but valuable information. I would like to resolve the issues on windows. Sigh, I really need to get a windows computer sometime soon :(

Could you describe what's happening on windows? My initial thought was line endings (\n vs \r\n), but the regex doesn't distinguish on that.

Could you try changing line 73 from

}).catch(e => [ defaultTarget ]);

to

}).catch(e => { console.log(e); return [ defaultTarget ] });

It will print any caught error to console. Make sure to open Dev tools (Toggle dev tools in command palette).

Thanks for your help!

[Leffmann]

Found it, split(os.EOL) will of course only work when the file is appropriated for the current system. Suggest replacing it with split(/[\r\n]+/) to make it filter all kinds of nonsensical line-breaks.

Close any time you want. Thanks again!

[noseglid]

Oh cool! Thanks!

So, you have your makefile with Unix line endings? Even under windows? Is that a requirement?

[noseglid]

I've pushed another commit which allows both \n and \r\n line endings. Does this work for you?

[Leffmann]

Yep, that fixed it, thanks :+1: