Update nix to 0.28.0

[omprakaash]

Updates nix to 0.28.0. Changed the deprecated functions to new recommended ones. Another noticeable change was the write() ~and close()~ function~s~ in nix now take in OwnedFd instead of RawFd. Included changes for them as well.

[codecov-commenter]

Codecov Report

Merging #2728 (1edc090) into main (18f3e0b) will decrease coverage by 0.02%. Report is 18 commits behind head on main. The diff coverage is 18.75%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2728      +/-   ##
==========================================
- Coverage   65.21%   65.20%   -0.02%     
==========================================
  Files         133      133              
  Lines       16981    16981              
==========================================
- Hits        11074    11072       -2     
- Misses       5907     5909       +2     

[utam0k]

Another noticeable change was the write() and close() functions in nix now take in OwnedFd instead of RawFd. Included changes for them as well.

Where can I find this change? I couldn't find the change you mentioned. https://github.com/nix-rust/nix/blob/master/CHANGELOG.md#changed

[omprakaash]

Yeah, I could not find that in the change-log too. The change was done in this PR: https://github.com/nix-rust/nix/pull/2100. Sorry, it seems that only write() has been changed. Close() still takes RawFd.

[utam0k]

@omprakaash It seems CI failed with lint. May I ask you to check it?

[omprakaash]

Oh sorry about that, should have fixed the linting issues now.

[utam0k]

@omprakaash Thanks!

[utam0k]

@zahash @anti-entropy123 @YJDoc2 May I ask you to leave this PR to you?

[YJDoc2]

Hey, thanks for updating. Two notes :

• there's conflict in lock file, you can recreate it or copy it over from master and run build to fix it
• Can you add explicit drop call where there was close before, and comment the reasoning, maybe linking this PR there? That way in future we know for sure that we have taken care of the fds and the reason for calling drop instead of close

[YJDoc2]

Hey @omprakaash , it seems there are still some more conflicts here. In case you are busy, is it ok if I resolve them and push? because the conflicts are in cargo* files, not code, it would be pretty straight forward to resolve. Apart from that, the code is lgtm.

[omprakaash]

Hey sorry for the late reply. I did not have the chance to look at this and missed your comment. Would be great if you could that. Thank you !

[yihuaf]

I will take care of this.

[yihuaf]

Merge conflict is resolved. I will look over the review comments over the weekend and address them.

[yihuaf]

@YJDoc2 This is ready for review one last time. I have addressed the merge conflicts and went over the reviews. One note, the closing of the pipes (The fds that were converted from RawFd to OwnedFd) is actually required. We use the pipes as both a way to communicate between processes and as a barrier for synchronization. The finer control to explicitly close is required, instead of letting the scope takes care of the close through Drop trait. By closing one end of the pipe, we immediately signal to the other end of the pipe, so that any blocking calls on the pipe will immediately return. The PR already included the change, but I changed the comments to reflect this information. The previous comment implies that the drop was there only to maintain the legacy code, but the explicit close actual serves a purpose in this case, not just because we have to manually close due to the use of RawFd.

Let me know if there is anything else you would like to make change.

[yihuaf]

close #2723