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

Fall cleanup #493

Merged
merged 34 commits into from
Sep 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
ef0e7df
Bump typescript to latest
niik Aug 18, 2022
e05e7f9
Build in more major version of node
niik Aug 18, 2022
b115a50
Require Node 12
niik Aug 18, 2022
9f56ff0
Switch to yarn
niik Aug 18, 2022
8c06616
Bump jest, ts-jest, and prettier
niik Aug 18, 2022
63b8158
Delete package-lock.json
niik Aug 18, 2022
e8c5ebe
Bump to latest tar
niik Aug 18, 2022
e228ede
cleanup
niik Aug 18, 2022
64c9f55
Download git without using Got
niik Aug 18, 2022
057e287
Calculate checksum without external module
niik Aug 18, 2022
5109d2c
Eh, https only is fine
niik Aug 18, 2022
a5bfeef
Remove Got from update-embedded-git
niik Aug 18, 2022
613939e
Actually install node version from matrix
niik Aug 18, 2022
994497b
One day I will learn how to be a real developer
niik Aug 18, 2022
f0bd3f2
Fix TS errors
niik Aug 18, 2022
613ca60
Revert prettier change to keep PR somewhat sane
niik Aug 18, 2022
23eae6a
Guess this is what we used
niik Aug 18, 2022
15b539e
Try using v3
niik Aug 18, 2022
577dcbc
Whoops
niik Aug 18, 2022
e78794a
Cancel in-progress jobs when pushing new changes
niik Aug 18, 2022
9ac759d
Update ci.yml
niik Aug 18, 2022
98433a8
Update ci.yml
niik Aug 18, 2022
7076ed1
Update ci.yml
niik Aug 18, 2022
566b19f
I don't know why this is necessary
niik Aug 18, 2022
55b3269
Replace mkdirp module for built-in recursive mkdir
niik Aug 18, 2022
3bb2259
Use built-in recursive remove and move rimraf to devDependencies
niik Aug 18, 2022
268bf4b
Whoops
niik Aug 18, 2022
08f7661
Use get utility function
niik Aug 18, 2022
7ff959f
'tis the default already
niik Aug 18, 2022
e29e338
ProgressBar will take care of this
niik Aug 18, 2022
a512d65
rmdirSync isn't available on Node 12
niik Aug 18, 2022
89ae017
It's called rm in Node 12
niik Aug 18, 2022
ff4b085
Update docs/releases.md
niik Aug 18, 2022
690f770
Bump ci
niik Aug 24, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 15 additions & 9 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,18 @@ on:
- main
pull_request:

concurrency:
group: ${{ github.ref }}
cancel-in-progress: true

jobs:
build:
name: ${{ matrix.friendlyName }} ${{ matrix.arch }}
name: ${{ matrix.friendlyName }} Node ${{matrix.nodeVersion}} ${{ matrix.arch }}
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
nodeVersion: [12, 14, 16, 18]
arch: [x64]
os: [macos-11, windows-2019, ubuntu-22.04]
include:
Expand All @@ -22,6 +27,7 @@ jobs:
friendlyName: Windows
- os: windows-2019
friendlyName: Windows
nodeVersion: 16
arch: x86
- os: ubuntu-22.04
friendlyName: Linux
Expand All @@ -30,23 +36,23 @@ jobs:
- uses: actions/checkout@v2
with:
submodules: recursive
- name: Install Node.js ${{ matrix.arch }}
uses: niik/setup-node@f9547c97f4c519f71dc42e652d6d2c53f9527c1d
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't notice we used a custom setup-node 🤯

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, and from some super untrustworthy source too!

- name: Install Node.js ${{ matrix.nodeVersion }} (${{ matrix.arch }})
uses: actions/setup-node@v3
with:
node-version: 12
node-arch: ${{ matrix.arch }}
node-version: ${{ matrix.nodeVersion }}
architecture: ${{ matrix.arch }}
- name: Install and build dependencies
run: npm ci
run: yarn
env:
npm_config_arch: ${{ matrix.arch }}
- name: Build
run: npm run build
run: yarn build
- name: Lint
run: npm run is-it-pretty
run: yarn is-it-pretty
- name: Reset safe.directory
run: git config --global --add safe.directory ""
shell: bash
- name: Test
run: npm run test
run: yarn test
env:
TEST: 1
30 changes: 18 additions & 12 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,20 @@ Add it to your project:
> npm install dugite
```

or

```
> yarn add dugite
```

Then reference it in your application:

```js
import { GitProcess, GitError, IGitResult } from 'dugite'

const pathToRepository = 'C:/path/to/git/repository/'

const result = await GitProcess.exec([ 'status' ], pathToRepository)
const result = await GitProcess.exec(['status'], pathToRepository)
if (result.exitCode === 0) {
const output = result.stdout
// do some things with the output
Expand All @@ -31,16 +37,16 @@ if (result.exitCode === 0) {

### Features

- make it easy to work with Git repositories
- use the same commands as you would in a shell
- access to the full set of commands, options and formatting that Git core uses
- access to the latest features of Git
- make it easy to work with Git repositories
- use the same commands as you would in a shell
- access to the full set of commands, options and formatting that Git core uses
- access to the latest features of Git

### Supported Platforms

- Windows 7 and later
- macOS 10.9 and up
- Linux (tested on Ubuntu Precise/Trusty and Fedora 24)
- Windows 7 and later
- macOS 10.9 and up
- Linux (tested on Ubuntu Precise/Trusty and Fedora 24)

### Status

Expand All @@ -52,7 +58,7 @@ If you are interested in getting involved with this project, refer to the [CONTR

As this is under active development, the roadmap is also subject to change. Some ideas:

- authentication support in-the-box
- make environment setup easier to override
- API additions for common tasks such as parsing output
- error handling improvements
- authentication support in-the-box
- make environment setup easier to override
- API additions for common tasks such as parsing output
- error handling improvements
14 changes: 8 additions & 6 deletions docs/releases.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
The most important part of the release process is updating the embedded Git package. This can be done using this one-liner:

```sh
npm run update-embedded-git
yarn update-embedded-git
```

This script:
Expand All @@ -23,12 +23,14 @@ const url = `https://api.github.com/repos/desktop/dugite-native/releases/2354453
```

## Release/Publishing

Before running the commands in 'Publishing to NPM',
create a new release branch of the form `releases/x.x.x`

After running commands in 'Publishing to NPM', the release branch should be pushed. Now, you need to get it reviewed and merged.

After that, don't forget publish the release on the repo.

- Go to https://github.com/desktop/dugite/releases
- Click click `Draft a New Release`
- Fill in form
Expand All @@ -39,12 +41,13 @@ After that, don't forget publish the release on the repo.
Releases are done to NPM, and are currently limited to the core team.

```sh
# to ensure everything is up-to-date and tests pass
npm ci
# to ensure everything is up-to-date
yarn

# if you have not run `npm run build` before, a couple of you cloning auth test will fail.
npm test
yarn build

# if you have not run `yarn build` before, a couple of you cloning auth test will fail.
yarn test

# you might need to do a different sort of version bump here
npm version minor
Expand All @@ -54,4 +57,3 @@ npm version minor
# remember to `npm login`
npm publish
```

9 changes: 4 additions & 5 deletions docs/setup.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@

This is what you need to install:

- [NodeJS](https://nodejs.org) v6 or higher
- [NodeJS](https://nodejs.org) v12 or higher

Then open a shell and clone the repository to your local machine.

To ensure everything is working, run this command from the root of the repository:

```sh
npm install
yarn
```

This will install the dependencies, compile the library source and run the suite of tests.
Expand All @@ -20,15 +20,14 @@ The coding style is based on [Prettier](https://github.com/prettier/prettier)
and can be validated from the command line:

```sh
npm run is-it-pretty
yarn is-it-pretty
```

To ensure your changes match the formatting, just run this before committing:

```sh
npm run prettify
yarn prettify
```

Or add [prettier integration for your editor](https://github.com/prettier/prettier#editor-integration)
for first-class support.

5 changes: 4 additions & 1 deletion lib/git-environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,10 @@ function resolveGitExecPath(): string {
*/
export function setupEnvironment(
environmentVariables: NodeJS.ProcessEnv
): { env: NodeJS.ProcessEnv; gitLocation: string } {
): {
env: NodeJS.ProcessEnv
gitLocation: string
} {
const gitLocation = resolveGitBinary()

let envPath: string = process.env.PATH || ''
Expand Down
17 changes: 11 additions & 6 deletions lib/git-process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,13 @@ export class GitProcess {

ignoreClosedInputStream(spawnedProcess)

if (options && options.stdin !== undefined) {
if (options && options.stdin !== undefined && spawnedProcess.stdin) {
// See https://github.com/nodejs/node/blob/7b5ffa46fe4d2868c1662694da06eb55ec744bde/test/parallel/test-stdin-pipe-large.js
spawnedProcess.stdin.end(options.stdin, options.stdinEncoding)
if (options.stdinEncoding) {
spawnedProcess.stdin.end(options.stdin, options.stdinEncoding)
} else {
spawnedProcess.stdin.end(options.stdin)
}
}

if (options && options.processCallback) {
Expand Down Expand Up @@ -313,18 +317,19 @@ export class GitProcess {
*
* See https://github.com/desktop/desktop/pull/4027#issuecomment-366213276
*/
function ignoreClosedInputStream(process: ChildProcess) {
function ignoreClosedInputStream({ stdin }: ChildProcess) {
// If Node fails to spawn due to a runtime error (EACCESS, EAGAIN, etc)
// it will not setup the stdio streams, see
// https://github.com/nodejs/node/blob/v10.16.0/lib/internal/child_process.js#L342-L354
// The error itself will be emitted asynchronously but we're still in
// the synchronous path so if we attempts to call `.on` on `.stdin`
// (which is undefined) that error would be thrown before the underlying
// error.
if (!process.stdin) {
if (!stdin) {
return
}
process.stdin.on('error', err => {

stdin.on('error', err => {
const code = (err as ErrorWithCode).code

// Is the error one that we'd expect from the input stream being
Expand All @@ -343,7 +348,7 @@ function ignoreClosedInputStream(process: ChildProcess) {
//
// "For all EventEmitter objects, if an 'error' event handler is not
// provided, the error will be thrown"
if (process.stdin.listeners('error').length <= 1) {
if (stdin.listeners('error').length <= 1) {
throw err
}
})
Expand Down
Loading