android / nowinandroid

A fully functional Android app built entirely with Kotlin and Jetpack Compose
Apache License 2.0
15.92k stars 2.81k forks source link

Fixed issue with module graphs not generating on Linux #1486

Closed neil3000 closed 2 days ago

neil3000 commented 1 month ago

What I have done and why

In commit e5008c655ea0373ece8c8dfef6e3aa073991249e a condition was added to check if the grep command supports the -P parameter.

However, from my testing at least, this doesn't seem to work as it uses /dev/null to check for "", probably based on the assumption that grep will return positively even if it doesn't find the matching string (here ""), which isn't true when using /dev/null.

At least that is my understanding of the issue, feel free to correct me if it isn't exactly that <3 What is sure however is that it doesn't work on my setup, even though I do have the grep -P option and it works fine (worked fine before e5008c655ea0373ece8c8dfef6e3aa073991249e, and still works when removing the condition).

Instead of doing a grep -P "" /dev/null I changed it to a echo "" | grep -P "", the result will still be positive as grep doesn't return an error when no matches are found, but the string isn't ""null"" as with /dev/null, and so the condition doesn't always return false.

dturner commented 2 days ago

Should be fixed with #1506. Please file an issue if not.

SimonMarquis commented 2 days ago

1506 did not change this grep command, and is still used here

https://github.com/android/nowinandroid/blob/644e618af1aaebbffd7f0b80e24e9e55b692b3c1/generateModuleGraphs.sh#L73-L74

This should probably be restored and merged. (not sure why the order of the error message has been changed though 😅)