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

Fix HDTcat to work under Windows #128

Merged
merged 5 commits into from
Aug 9, 2021
Merged

Conversation

fkleedorfer
Copy link
Contributor

  • Adds close() methods to LongArrayDisk and dependent classes. Also makes them implement Closeable. Closing LongArrayDisk causes the underlying RandomAccessFile to be closed, which was never done, leading to exceptions being thrown when those files were to be removed.
  • Removes the setLength() call in LongArrayDisk, which also causes an exception under Windows. Nowhere in the docs is it stated that this call is necessary, the file only grows upon writing to it. The effect (under Windows) is simply that the exception goes away.
  • Adds missing close() calls for some streams in FourSectionDictionaryCat
  • Replaces Files.delete(Paths.get(...)) calls with new File(...).delete() calls, which inexplicably gets rid of the exceptions thrown under Windows when temporary files are deleted.

Fixes #127

* Adds `close()` methods to LongArrayDisk and dependent classes. Also makes them closeable. Closing LongArrayDisk causes the underlying RandomAccessFile to be closed, which was never done, leading to exceptions being thrown when those files were to be removed.
* removes the setLength() call in LongArrayDisk, which also causes an exception under Windows. Nowhere in the docs is it stated that this call is necessary, the file only grows upon writing to it. The effect (under Windows) is simply that the exception goes away.
* Adds missing close() calls for some streams in FourSectionDictionaryCat
* Replaces `Files.delete(Paths.get(...))` calls with `new File(...).delete()` calls, which inexplicably gets rid of the exceptions thrown under Windows when temporary files are deleted.
@D063520
Copy link
Contributor

D063520 commented Jul 13, 2021

I overflew the code and I run the tests. For me we can accept this pull request.
@fkleedorfer Thank you very much for this contribution!
PS: I did not check it under windows but I assume it is working there now ....

@D063520
Copy link
Contributor

D063520 commented Jul 13, 2021

@fkleedorfer can you also remove this now ....

System.out.println("NOTE: this tool is not working under WINDOWS! This is a well-known BUG!");

@fkleedorfer
Copy link
Contributor Author

done

@mielvds
Copy link
Member

mielvds commented Jul 14, 2021

Looks good. Anything else before I merge?

@D063520
Copy link
Contributor

D063520 commented Jul 14, 2021

For me it is fine!

@mielvds mielvds self-requested a review July 15, 2021 07:48
@mielvds
Copy link
Member

mielvds commented Jul 15, 2021

Thanks both! I asked some minor questions, mostly about changes made to the core code

@D063520
Copy link
Contributor

D063520 commented Jul 15, 2021

Basically there is no change to the core code. The code changes affect only hdtCat and the tests are passing. But it's good that you give a look too!

@fkleedorfer
Copy link
Contributor Author

I asked some minor questions, mostly about changes made to the core code

Not sure where I can see these questions, @mielvds

@fkleedorfer
Copy link
Contributor Author

From my side, this PR is ready to merge.

@mielvds mielvds merged commit 2f22964 into rdfhdt:master Aug 9, 2021
@mielvds
Copy link
Member

mielvds commented Aug 9, 2021

Thanks @fkleedorfer @D063520 !

@mielvds mielvds added this to the 2.1.3 milestone Aug 9, 2021
ate47 added a commit to ate47/hdt-java that referenced this pull request Jul 9, 2022
…anager#indexedHDT(), remove many unhandled try/catch, remove unhandled try/catch added in rdfhdt#128 and fix LongArrayDisk if sizeBit % MAPPING_SIZE == 0
@ate47 ate47 mentioned this pull request Jul 9, 2022
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.

Can we make HDTCat work under windows?
3 participants