Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

removed a bashism that prevented configuration #3902

Closed
wants to merge 1 commit into from

Conversation

oliverepper
Copy link
Contributor

This time against aconfigure.ac.

@sauwming
Copy link
Member

  • I think it needs to be more complete than this, because just right above it there are quite a few that still uses +=.

  • And to simplify it, instead of testing if [ -n "${ac_webrtc_aec3_cflags}" ]; everytime, perhaps it's simpler to just do one time initialisation of ac_webrtc_aec3_cflags="".

  • Finally, the generated aconfigure also needs to be included in the PR.

@oliverepper
Copy link
Contributor Author

I know there are a lot more += bashisms in the file, but this was the one preventing aconfigure from running on FreeBSD. The only one that needed fixing after adding | *amd64*freebsd* | *x86_64*freebsd* to the case statement. (Maybe I should include this, too).

There is no need to pre-initialise a variable in sh. You can simply do:

DEMO="${MAYBE_NON_EXISTANT} -Dimportant"

echo "${DEMO}"

I did the test to prevent the unnecessary and ugly whitespace in front of -Dimportant.

I have a newer version of autoconf installed. If I generate the aconfigure script there will be a bunch of unrelated changes.

Please allow me to derail this:
I am in the process of maintaining/updating the pjproject port for FreeBSD. I have a working port already, but a few things are not yet configurable. Giving the option --enable-libwebrtc-aec3 does not work right away. The above is one of the patches that are necessary. A few types must be adjusted, too. (pid_t is 4 bytes on FreeBSD).

Can we switch to email and discuss if a completely working port (all possible configurations) for FreeBSD is desirable and if I should create a branch to upstream all the necessary changes?

@sauwming
Copy link
Member

You don't need to fix the entire file, but at least all the ac_webrtc_aec3_cflags, just to be consistent?

Sure, please feel free to drop us an email.

@oliverepper
Copy link
Contributor Author

Ok. Perfect. Let's close this for now and I send you an email with a few questions and I will create a branch for what needs fixing for FreeBSD.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants