Remove now-unnecessary poll, fcntl includes from net(base).cpp #27530

1. 
   Empact commented at 8:36 PM on April 25, 2023: member

   As far as I can tell, the code calling for these includes was removed in: 6e68ccbefea6509c61fc4405a391a517c6057bb0 #24356 82d360b5a88d9057b6c09b61cd69e426c7a2412d #21387

2. 
   DrahtBot commented at 8:36 PM on April 25, 2023: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	fanquake
   Concept ACK	ccdle12
   Stale ACK	jonatack

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #27375 (net: support unix domain sockets for -proxy and -onion by pinheadmz)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

3. 
   maflcko commented at 2:05 PM on April 26, 2023: member

   Can you add this to iwyu CI to double check and aid reviewers?

4. maflcko added the label Refactoring on Apr 26, 2023
5. 
   ccdle12 commented at 3:00 PM on April 27, 2023: contributor

   tACK

   <details><summary>I also ran netbase.cpp through the iwyu CI and seems like there maybe a few more unused imports, but I haven't thoroughly tested removing the other ones</summary> <p>

   netbase.cpp should remove these lines:
   - #include <fcntl.h>  // lines 25-25
   - #include <poll.h>  // lines 29-29
   - #include <chrono>  // lines 18-18
   - #include <cstdint>  // lines 19-19
   - #include <limits>  // lines 21-21
   

   </p> </details>

6. 
   jonatack commented at 3:06 PM on April 27, 2023: contributor

   Can you add this to iwyu CI to double check and aid reviewers?

   I did that recently for these files while working on #27385; the results are here: https://cirrus-ci.com/task/6749578737745920

7. 
   jonatack commented at 3:10 PM on April 27, 2023: contributor

   utACK b095ac573b89d6898168bb9fda2abf7622ddec09

   Noticed these while doing #27385. Per https://cirrus-ci.com/task/6749578737745920:

   net.cpp should remove these lines:
   - #include <clientversion.h>  // lines 15-15
   - #include <crypto/sha256.h>  // lines 18-18
   - #include <fcntl.h>  // lines 45-45
   - #include <math.h>  // lines 63-63
   - #include <poll.h>  // lines 53-53
   - #include <array>  // lines 57-57
   - #include <unordered_map>  // lines 61-61
   
   netbase.cpp should remove these lines:
   - #include <fcntl.h>  // lines 26-26
   - #include <poll.h>  // lines 30-30
   - #include <chrono>  // lines 19-19
   - #include <cstdint>  // lines 20-20
   - #include <limits>  // lines 22-22
   

   Edit: #include <fcntl.h> should be added to util/sock.cpp

8. bitcoin deleted a comment on Apr 28, 2023
9. bitcoin deleted a comment on Apr 28, 2023
10. 
    Empact commented at 7:51 AM on April 28, 2023: member

    I'll update this PR on Sunday, just busy with http://btcpp.dev at the moment.

11. maflcko added the label Waiting for author on Apr 28, 2023
12. maflcko removed the label Waiting for author on Apr 28, 2023
13. Empact force-pushed on Apr 29, 2023
14. DrahtBot added the label CI failed on Apr 29, 2023
15. DrahtBot removed the label CI failed on Apr 29, 2023
16. DrahtBot added the label CI failed on Apr 30, 2023
17. Empact force-pushed on May 1, 2023
18. DrahtBot removed the label CI failed on May 1, 2023
19. 
    fanquake commented at 3:26 PM on May 9, 2023: member

    The scope of this has expanded, and it now conflicts with a number of things, that are much higher priority. You could wait until all the kernel / assumeutxo changes are merged, (and maybe also 27571, to remove the need to update the list), or, revert back to the original non-conflicting change, and that could probably be merged. Otherwise, I think this is likely to just sit for some time, and mostly generate merge-conflict noise in other drahtbot/other PRs.

20. DrahtBot added the label Needs rebase on May 10, 2023
21. Remove now-unnecessary poll, fcntl includes from net(base).cpp

    As far as I can tell, the code calling for these includes was removed in:
    6e68ccbefea6509c61fc4405a391a517c6057bb0 #24356
    82d360b5a88d9057b6c09b61cd69e426c7a2412d #21387

    8d9b90a61e
22. Empact force-pushed on Jun 28, 2023
23. DrahtBot removed the label Needs rebase on Jun 28, 2023
24. fanquake approved
25. 
    fanquake commented at 9:32 AM on June 29, 2023: member

    ACK 8d9b90a61e3a2a451abfce25328d13aa1e8f749b

26. DrahtBot requested review from jonatack on Jun 29, 2023
27. fanquake merged this on Jun 29, 2023
28. fanquake closed this on Jun 29, 2023

    ---

29. Empact deleted the branch on Jun 29, 2023
30. sidhujag referenced this in commit 79b064aa05 on Jun 30, 2023
31. bitcoin locked this on Jun 28, 2024

Review Requested
jonatack