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

Prevent dangling symlink creation #4644

Merged
merged 1 commit into from
Oct 9, 2021

Conversation

anna-kartynnik
Copy link
Contributor

When $srcdir is a relative path, the path written by ln is invalid w.r.t. the symlink directory. Use an absolute path instead.

anna-kartynnik referenced this pull request Oct 7, 2021
There are presumably places other than utils/subset_data_dir.sh that
can use this.

utt2num_frames can be produced by passing --write_utt2num_frames=true
to steps/make_mfcc.sh. This is much faster than running feat-to-len
after the fact.
@@ -116,7 +116,7 @@ else
# Select $numutt shortest utterances.
. ./path.sh
if [ -f $srcdir/utt2num_frames ]; then
ln -sf $srcdir/utt2num_frames $destdir/tmp.len
ln -sf $(readlink -f $srcdir)/utt2num_frames $destdir/tmp.len
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The -f option to readlink does not work on a mac. I think it would be better to use utils/make_absolute.sh instead of readlink -f.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I didn't know support for BSD-like systems is intended, because searching for readlink -f had quite a few hits. Will use make_absolute.sh.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, some dataset-specific scripts still have that, but it's discouraged. thanks!

When `$srcdir` is a relative path, the path written by `ln` is invalid w.r.t. the symlink directory. Use an absolute path instead.
@danpovey danpovey merged commit 90598e2 into kaldi-asr:master Oct 9, 2021
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