fix(main/predict): prevent opportunistic inclusion of asoundlib.h

[robertkirkman]

Fixes #22048

[licy183]

Suggested change

	+ #if 0
	+ #if __has_include(<alsa/asoundlib.h>) && !defined(__ANDROID__)

[robertkirkman]

I wouldn't say that way of writing it is my personal preference in this situation, but I do understand it can make the purpose more readable and clear to others so I will change it to that, sure!

I just don't think that the upstream developer should copy and paste this patch directly into their code if they see it. I think they should move around some of their code blocks in a different way in order to make future release archives not have this problem, but writing the patch for the current version of this package with only 2 lines of changes keeps the patch file small. so my #if 0 was intended to be a instruction to the upstream developer to not copy the patch and instead think about how to write their fix a way that involves a larger diff, but now I have written this comment explaining, so if the upstream developer sees my comment, they will get the information I am trying to convey in a more explicit way, so I can change it to __has_include(<alsa/asoundlib.h>) && !defined(__ANDROID__), just for the patch.

Alternatively: I could also write the change I believe the upstream developer should commit if they also believe alsa-lib will continue to not provide a functioning snd_pcm_open() on Android in the future. That would result in a much larger and different-looking .patch file. You can let me know if you think that would be the best idea instead.

[licy183]

If you have this concern, it is more appropriate to use __has_include(<alsa/asoundlib.h>) && !defined(__TERMUX__).

[robertkirkman]

Sorry I might not have been very clear, specifically what I meant is that I believe the upstream developer should do this logical pattern:

--- a/predict.c
+++ b/predict.c
@@ -38,10 +38,10 @@
 #include "predict.h"
 
 #if defined __has_include
-  #if __has_include(<alsa/asoundlib.h>)
-	#include <alsa/asoundlib.h>
-  #elif defined (__ANDROID__)
+  #if defined (__ANDROID__)
 	char wavestring[1024];
+  #elif __has_include(<alsa/asoundlib.h>)
+	#include <alsa/asoundlib.h>
   #endif
 #endif
 

That is to say, check for __ANDROID__ first, and then only attempt to detect asoundlib.h if __ANDROID__ is not detected. But if I apply that order in the patch right now, the patch to the second Hunk will get bigger and harder to read.

[robertkirkman]

if #20483 one day in the future becomes able to make a functioning, root-free implementation of the snd_pcm_open() function on Android by extending Pipewire to be able to handle that use case, then in that case what I said about reordering the code blocks would not be relevant anymore, so that is actually a point in favor of a third option, which is to keep the patch exactly as it is now and deliberately not upstream it (to the predict release tarball), because if a future termux package did actually provide snd_pcm_open(), it could then be activated in predict by simply removing the patch, to automatically activate it. That's a lot of hypothetical possibilities though, so it's hard to guess what the most likely situation will look like 2 or 4 years in the future. To me it seems unlikely because I don't think upstream Pipewire can normally ever "fully emulate" an ALSA sound card but, maybe anything is possible.

[licy183]

Suggested change

	+ #if 0
	+ #if __has_include(<alsa/asoundlib.h>) && !defined(__ANDROID__)

[twaik]

LGTM.