FS constants says not all constants are available on all platforms, but it doesn't say which ones are available on what platforms

[dead-claudia]

Affected URL(s)

https://nodejs.org/api/fs.html#fsconstants

Description of the problem

It says the following:

Not every constant will be available on every operating system.

However, this doesn't say anything else about the availability of various constants. In particular, none of the file mode constants are available on Windows and about half of the file type constants are also not present, but the docs don't even so much as hint at this. (I initially thought this to be a bug and filed #41590, so it's definitely confusing.) Part of what makes it non-obvious is the fact many of the various Unix-based constants (both in fs and elsewhere like with many signals) are shimmed on Windows in terms of corresponding Windows APIs, and so I can't just assume for all of them. (In particular, fs.constants.O_DIRECTORY is missing from Windows despite fs.opendir using Windows APIs similar to POSIX's opendir - that one was very surprising.)

Could the series of tables be updated to include platform availability for each relevant constant?

[ilg-ul]

I've also got bitten by the missing S_IWUSR on Windows.

I think that this particular constant must be defined for Windows, since chmod() is able to set this bit; perhaps other constants are in this category too.

The constants not used on Windows can be omitted, but the documentation must explicitly mention this.

[bnoordhuis]

I've added the "good first issue" label because it just needs someone to go and open a pull request.

---

@ilg-ul S_IWUSR is missing on Windows because it's called _S_IWRITE there (which node doesn't export.)

You could open a pull request exporting _S_IWRITE and _S_IREAD, or aliasing them to S_IWUSR and S_IRUSR.

[ilg-ul]

I took a look, and I identified in node_constants.cc several definitions that add constants. Is this the correct location?

If so, I suggest we alias the definitions like this:

#if defined(_WIN32)
#include <io.h>  // _S_IREAD _S_IWRITE
#ifndef S_IRUSR
#define S_IRUSR _S_IREAD
#endif  // S_IRUSR
#ifndef S_IWUSR
#define S_IWUSR _S_IWRITE
#endif  // S_IWUSR
#endif

Is this what you mean?

[bnoordhuis]

Yes, that would work. Variations of that pattern are already in use elsewhere in the code base.

[ilg-ul]

I added a small test to verify that the definitions are generated on Windows (the test passed), and I also added some explicit mentions in the documentation.

[ilg-ul]

The CI checks passed.

Do I have to do anything more, or simply wait for a review?

[ilg-ul]

I created a new PR, hopefully cleaner: #42757.

[dead-claudia]

@ilg-ul Could you fix the description of #42757 such that it doesn't close this issue? It doesn't actually fix this as this extends well beyond those two constants.

[ilg-ul]

I replaced Fixes with Refs. Is this ok?

Btw, the patch also changes the documentation to mention which constants are available on Windows. Isn't this what you asked for?

[dead-claudia]

@ilg-ul

Btw, the patch also changes the documentation to mention which constants are available on Windows. Isn't this what you asked for?

I didn't realize that - didn't see it in the summary and I didn't actually look at the patch in question. In that case, this can be resolved by that.