GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — process: add O_NOATIME constant by Trott · Pull Request #6492 · nodejs/node

Conversation

@Trott

@Trott Trott commented Apr 30, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
Checklist
  • tests and code linting passes
  • a test and/or benchmark is included
  • the commit message follows commit guidelines
Affected core subsystem(s)

process src test

Description of change

Add O_NOATIME fs flag on Linux.

WIP. Do not land (yet). (Running the test first everywhere to make sure results are as expected on all the Linux hosts.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Trott Trott added wip Issues and PRs that are still a work in progress. process Issues and PRs related to the process subsystem. labels Apr 30, 2016
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to Node.js core tests and test infrastructure. label Apr 30, 2016
@Trott
process: add O_NOATIME constant
cf4c83c 
Add O_NOATIME flag on Linux for use with `fs.open()`.

PR-URL: nodejs#6492
Fixes: nodejs#2182
@Trott
Trott force-pushed the noatime branch from 560db96 to cf4c83c Compare April 30, 2016 22:38
@Trott Trott removed the wip Issues and PRs that are still a work in progress. label Apr 30, 2016
@Trott

Trott commented Apr 30, 2016

Copy link
Copy Markdown
Member Author

Removed in-progress label. Ready for review.

CI: https://ci.nodejs.org/job/node-test-pull-request/2447/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Trott Trott mentioned this pull request Apr 30, 2016
Add support for O_NOATIME flag to fs.open #2182
Closed
@jasnell jasnell added wip Issues and PRs that are still a work in progress. and removed wip Issues and PRs that are still a work in progress. labels May 1, 2016
@jasnell

jasnell commented May 1, 2016

Copy link
Copy Markdown
Member

ha! saw the in progress comment in the PR description, added the label, then scanned lower ;-)

would it be possible to add a test that uses the constant? it's always concerned me that these things aren't documented that well :-(
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@addaleax

addaleax commented May 1, 2016

Copy link
Copy Markdown
Member

would it be possible to add a test that uses the constant?

Unfortunately not in a reliable way, the flag is only advisory to the OS/the filesystem.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell

jasnell commented May 1, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Didn't think so but didn't hurt to ask ;). Probably the only objection I
can think of on this is that constants is still not documented properly.

It's likely too late to do so now but ideally these kinds of constants
would be exposed via the module in which they are relevant (i.e. fs related
constants on the fs module, crypto related constants on the crypto module).
Add in proper documentation for them, then deprecate require('constants')
entirely (keeping the internal process.binding('constants') tho. Ah well.
On Apr 30, 2016 7:24 PM, "Anna Henningsen" [email protected] wrote:

would it be possible to add a test that uses the constant?

Unfortunately not in a reliable way, the flag is only advisory to the
OS/the filesystem.

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#6492 (comment)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bnoordhuis

bnoordhuis commented May 1, 2016

Copy link
Copy Markdown
Member

LGTM
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jbergstroem

jbergstroem commented May 1, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

O_NOATIME arrived in linux 2.6.8 it seems. Is there a (supported) Linux distribution that we should care about? I couldn't find anything.

Edit: Some more pointless googling, treat my comment as noise.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bnoordhuis

bnoordhuis commented May 1, 2016

Copy link
Copy Markdown
Member

2.6.18 (RHEL 5) is the baseline, it was 2.6.9 (RHEL 4) before that.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell jasnell mentioned this pull request May 1, 2016
doc: add initial constants module documentation #6505
Closed
1 task
@Trott

Trott commented May 3, 2016

Copy link
Copy Markdown
Member Author

CI: https://ci.nodejs.org/job/node-test-pull-request/2470/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell

jasnell commented May 3, 2016

Copy link
Copy Markdown
Member

Related: #6534
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@evanlucas

evanlucas commented May 3, 2016

Copy link
Copy Markdown
Contributor

should constants be the subsystem?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell

jasnell commented May 3, 2016

Copy link
Copy Markdown
Member

src: would likely be more appropriate.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell jasnell added c++ Issues and PRs that require attention from people who are familiar with C++. semver-minor PRs that contain new features and should be released in the next minor version. labels May 3, 2016
@jasnell

jasnell commented May 3, 2016

Copy link
Copy Markdown
Member

LGTM
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Trott added a commit to Trott/io.js that referenced this pull request May 4, 2016
@Trott
src: add O_NOATIME constant
52f85be 
Add O_NOATIME flag on Linux for use with `fs.open()`.

PR-URL: nodejs#6492
Fixes: nodejs#2182
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@Trott

Trott commented May 4, 2016

Copy link
Copy Markdown
Member Author

Landed in 52f85be
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Trott Trott closed this May 4, 2016
@thefourtheye thefourtheye mentioned this pull request May 6, 2016
test: make sure O_NOATIME is present only in Linux #6614
Closed
3 tasks done
evanlucas pushed a commit that referenced this pull request May 17, 2016
@Trott @evanlucas
src: add O_NOATIME constant
37736f4 
Add O_NOATIME flag on Linux for use with `fs.open()`.

PR-URL: #6492
Fixes: #2182
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@evanlucas evanlucas mentioned this pull request May 17, 2016
2016-05-17, Version 6.2.0 (Stable) #6810
Merged
evanlucas added a commit that referenced this pull request May 17, 2016
@evanlucas
2016-05-17, Version 6.2.0 (Stable)
a4705f6 
- **buffer**: fix lastIndexOf and indexOf in various edge cases (Anna
  Henningsen) [#6511](#6511)
- **child_process**: use /system/bin/sh on android (Ben Noordhuis)
  [#6745](#6745)
- **deps**:
  - upgrade npm to 3.8.9 (Rebecca Turner)
    [#6664](#6664)
  - upgrade to V8 5.0.71.47 (Ali Ijaz Sheikh)
    [#6572](#6572)
  - upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé)
    [#6796](#6796)
  - Intl: ICU 57 bump (Steven R. Loomis)
    [#6088](#6088)
- **repl**:
  - copying tabs shouldn't trigger completion (Eugene Obrezkov)
    [#5958](#5958)
  - exports `Recoverable` (Blake Embrey)
    [#3488](#3488)
- **src**: add O_NOATIME constant (Rich Trott)
  [#6492](#6492)
- **src,module**: add --preserve-symlinks command line flag (James M
  Snell) [#6537](#6537)
- **util**: adhere to `noDeprecation` set at runtime (Anna Henningsen)
  [#6683](#6683)

As of this release the 6.X line now includes 64-bit binaries for Linux
on Power Systems running in big endian mode in addition to the existing
64-bit binaries for running in little endian mode.

PR-URL: #6810
evanlucas added a commit that referenced this pull request May 17, 2016
@evanlucas
2016-05-17, Version 6.2.0 (Stable)
26120e2 
- **buffer**: fix lastIndexOf and indexOf in various edge cases (Anna
  Henningsen) [#6511](#6511)
- **child_process**: use /system/bin/sh on android (Ben Noordhuis)
  [#6745](#6745)
- **deps**:
  - upgrade npm to 3.8.9 (Rebecca Turner)
    [#6664](#6664)
  - upgrade to V8 5.0.71.47 (Ali Ijaz Sheikh)
    [#6572](#6572)
  - upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé)
    [#6796](#6796)
  - Intl: ICU 57 bump (Steven R. Loomis)
    [#6088](#6088)
- **repl**:
  - copying tabs shouldn't trigger completion (Eugene Obrezkov)
    [#5958](#5958)
  - exports `Recoverable` (Blake Embrey)
    [#3488](#3488)
- **src**: add O_NOATIME constant (Rich Trott)
  [#6492](#6492)
- **src,module**: add --preserve-symlinks command line flag (James M
  Snell) [#6537](#6537)
- **util**: adhere to `noDeprecation` set at runtime (Anna Henningsen)
  [#6683](#6683)

As of this release the 6.X line now includes 64-bit binaries for Linux
on Power Systems running in big endian mode in addition to the existing
64-bit binaries for running in little endian mode.

PR-URL: #6810
@MylesBorins MylesBorins added the dont-land-on-v4.x label Jun 2, 2016
@Trott
Trott deleted the noatime branch January 13, 2022 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. process Issues and PRs related to the process subsystem. semver-minor PRs that contain new features and should be released in the next minor version. test Issues and PRs related to Node.js core tests and test infrastructure.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

8 participants

@Trott @jasnell @addaleax @bnoordhuis @jbergstroem @evanlucas @MylesBorins @nodejs-github-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.