search

kaldi-asr / kaldi

kaldi-asr/kaldi is the official location of the Kaldi project.
http://kaldi-asr.org
Other
14.25k stars 5.32k forks source link

validate_data_dir.sh checks for empty variable which already exists in parent environment #4511

Closed Sentewolf closed 3 years ago

Sentewolf commented 3 years ago

In #4094 a check is introduced in validate_data_dir.sh to see if $data is an empty string. This check always fails if validate_data_dir.sh is called by make_mfcc.sh

It seems to me we should not check if $data is an empty string in validate_data_dir.sh.

Reverting a few lines in validate_data_dir.sh fixes the issue.

I tested this on RedHat and Debian. As far as I can tell, using the parent environment is the normal way for a shell to work so I expect this to fail in most environments.

Before I make a pull request to revert the changes made in #4094, I wanted to check why this extra validation was added, if it is needed in some other workflow, and if another way can be found to implement it.

danpovey commented 3 years ago

What is your OS and what is your shell? Unless you do export data, it should not be available to the child process's shell.

Sentewolf commented 3 years ago

My apologies for wasting your time, you are completely right. It's shell scripts all the way down and somewhere up the chain was a "set -a" which I just learned exports variables.

I guess I'll have to try and fix a few other repositories (though the added check in validate_data_dirs.sh still seems a bit unnecessary).

danpovey commented 3 years ago

I think it was using that method to check for repeated command line parameters.

On Fri, Apr 30, 2021 at 11:40 PM Sentewolf @.***> wrote:

My apologies for wasting your time, you are completely right. It's shell scripts all the way down and somewhere up the chain was a "set -a" which I just learned exports variables.

I guess I'll have to try and fix a few other repositories (though the added check in validate_data_dirs.sh still seems a bit unnecessary).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kaldi-asr/kaldi/issues/4511#issuecomment-830182525, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZFLO6B5EGZKCQ6PHOV32TTLLFODANCNFSM434JZCBA .