BuildBamIndex creates index file in the current directory

[ghost]

Dear colleagues,

I discovered a thing which seems invalid. BuildBamIndex creates index file in the current directory, not in the directory which contains bam file. I'm absolutely sure that it behaved as I expected in previous versions.

Example:

source ~/miniconda3/bin/activate gatk
cd /some/path
gatk BuildBamIndex -I /path/to/bam/my_file.bam

Expected output:

/path/to/bam/my_file.bai

Observed:

/some/path/my_file.bai

OS: Ubuntu 22.04
GATK version: 4.2.6.1
Environment: miniconda3

Could you check that, please?

[cmnbroad]

Yes, this is probably a regression introduced in Picard #1787. Im transferring this to the Picard repo.

[cmnbroad]

This was originally reported in GATK, but the issue is in BuildBamIndex.

[cmnbroad]

@regnveig To work around this for now, I think it will work correctly if you specify the full path of the expected output using -O.

[ghost]

@cmnbroad thank you for your suggestion! Looking forward to the fixed version!

[kachulis]

will be fixed by #1852

[rickymagner]

Looks like this was fixed by #1852.

[cmnbroad]

I think there was a lot of confusion around all the PRs related to cloud-enabling. This will be fixed by cloud-enabling BuildBamIndex, but I don't see any evidence that supports that having been done in #1852.

[kockan]

@cmnbroad Is this intended to be part of #1804 ? If not, would it be a simple matter of updating the input/output paths from File to PicardHtsPath (and the corresponding path conversions in the BuildBamIndex source), mirroring how @takutosato seems to have been doing it? I could give it a try, or just wait until #1804 is merged if preferred.

[cmnbroad]

@kockan Ah, this one has had a bit of a tortured lifecycle. BuildBamIndex already uses PicardHtsPath for inputs, but the outputs weren't done simultaneously, and they really should have been, because in some code paths the tool has to derive the output filename from the input filename.

I had a branch that fixes that, but it was abandoned because we didn't have the test infrastructure that we needed to add a test that uses an actual cloud path, so we decided instead to include a fix for this as part of #1804 (and also, IIRC, I did try to add a test that exercised the new code, but on a local file, but I started to notice that the existing test had some issues).

But in the meantime, #1804 has grown a bit and now has it's own tortured existence, and this fix isn't in there. So rather than add another tax on #1804 for that, I'll resurrect my branch with the fix, make a draft PR, and then when #1804 goes in we can add the new test.