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

Added: Extensions for Nx Library And Support for Writing to Memory Mapped Files #55

Merged
merged 6 commits into from
Jul 30, 2024

Conversation

Sewer56
Copy link
Member

@Sewer56 Sewer56 commented Jul 29, 2024

This PR adds the ability to write to Memory Mapped Files.

In addition, there are extensions which introduce support for IOutputDataProvider and IFileDataProvider from the Nx archive library. These are in an 'Extensions' package.

This will allow us to run Garbage Collection tests in memory.

@Sewer56 Sewer56 added the area-data-store This is related to the Data Store. label Jul 29, 2024
@Sewer56 Sewer56 requested a review from a team July 29, 2024 17:44
@Sewer56 Sewer56 self-assigned this Jul 29, 2024
@erri120
Copy link
Member

erri120 commented Jul 29, 2024

In addition, there are extensions which introduce support for IOutputDataProvider and IFileDataProvider from the Nx archive.

What are those doing here? They should be in the NX or App repo.

@Sewer56
Copy link
Member Author

Sewer56 commented Jul 29, 2024

They should be in the NX or App repo.

No matter what I do here, someone will not be happy.

It ideally should be tied in its own repo, because our workflows are not suited for situations where you only want to release some packages that exist in a repository. That is what I personally would prefer to do.

Putting it in any other repository would just lead to either having to upload the package manually (which I don't mind, changes would be rare), or having millions of different versions of the package with literally no changes.

Alternatively, we need a system that doesn't tie the package version to tag version for all projects.

I put it in a separate project here, as I figured it would be the location that would receive the least complaints. I don't want you complaining that I'm putting too much effort again. And also, because it needs an updated NexusMods.Paths, so you could release both at once.

Anyway, brb, I gotta reboot to (and reinstall, because it's broken) Windows and fix a case of seemingly open handles.


RE. Nx repo.

That's stuck waiting in limbo until this is merged.
- With this I can reintegrate the xxHash lib as a dependency and not use my own modified copy.
- I am waiting for this before I make the release that drops older runtimes.
- And without this I cannot reference Paths as a dependency because NuGet.Build.props sets the versions for all projects at the moment.

I just don't want to be a nuisance; so I haven't bugged anyone about it as it's not urgent.

@halgari
Copy link
Contributor

halgari commented Jul 29, 2024

It's not super awesome that the details of other libraries are leaking into .Paths, it's somewhat due to OSes not providing common abstractions over memory mapped files, and so if we want to provide abstractions this is probably the most logical place for it, especially if we want testable interfaces. So I'm 👍 on this one.

@halgari
Copy link
Contributor

halgari commented Jul 29, 2024

Ping me when the tests pass and I'll review it quickly

@Sewer56 Sewer56 force-pushed the nx-extensions branch 4 times, most recently from d369ddf to 0781561 Compare July 30, 2024 00:09
@Sewer56
Copy link
Member Author

Sewer56 commented Jul 30, 2024

Reinstalling windows (since mine was borked) and setting stuff up took a bit longer than expected. 😅

In any case, just a small commit, I just needed to dispose handles earlier since I run the tests on both Real and InMemory FS. So cleanup (file delete) couldn't happen while it was technically 'in use'.

Good to review.

I think tomorrow morning I'll add extension methods for NxRepackerBuilder and NxUnpackerBuilder for convenience into the Nx Extensions package in separate PR. Then it'll be a-ok for release.

@Sewer56 Sewer56 merged commit 3f26d06 into main Jul 30, 2024
5 checks passed
@Sewer56 Sewer56 deleted the nx-extensions branch July 30, 2024 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-data-store This is related to the Data Store.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants