`np` does not actually ensure "that there are no unpulled changes"

[janpio]

Description

The README states that np "Ensures ... that there are no unpulled changes". This is only true if the local checkout already knows that there are remote changes that could be pulled because this information was fetched.

Look at this failure:

λ np minor --no-publish

  √ Git
  √ Cleanup
  √ Installing dependencies using npm
  √ Running tests using npm
  √ Bumping version using npm
  × Pushing tags
    → hint: See the 'Note about fast-forwards' in 'git push --help' for details.

× Command failed: git push --follow-tags
To https://github.com/apache/cordova-plugin-device
 * [new tag]         v2.1.0 -> v2.1.0
 ! [rejected]        master -> master (fetch first)
error: failed to push some refs to 'https://github.com/apache/cordova-plugin-device'
hint: Updates were rejected because the remote contains work that you do
hint: not have locally. This is usually caused by another repository pushing
hint: to the same ref. You may want to first integrate the remote changes
hint: (e.g., 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

The check that "Ensures ... that there are no unpulled changes" runs this command and fails if something other than 0 is returned:

E:\Projects\Cordova\cordova-plugin-device (master -> origin) ([email protected])
λ git rev-list --count --left-only @{u}...HEAD
0

As you can see this actually does return 0 for me locally. But:

E:\Projects\Cordova\cordova-plugin-device (master -> origin) ([email protected])
λ git fetch origin
remote: Enumerating objects: 6, done.
remote: Counting objects: 100% (6/6), done.
remote: Compressing objects: 100% (6/6), done.
remote: Total 6 (delta 2), reused 0 (delta 0), pack-reused 0
Unpacking objects: 100% (6/6), done.
From https://github.com/apache/cordova-plugin-device
   61ad76c..594fa61  master     -> origin/master

E:\Projects\Cordova\cordova-plugin-device (master -> origin) ([email protected])
λ git rev-list --count --left-only @{u}...HEAD
2

After fetching from origin, I now have 2 unpulled changes and np will fail as it should:

λ np --no-publish

Publish a new version of cordova-plugin-device (current: 2.2.0)

? No commits found since previous release, continue? Yes
? Select semver increment or specify new version minor  2.3.0

  > Git
    √ Check current branch
    √ Check local working tree
    × Check remote history
      → Remote history differs. Please pull changes.
    Cleanup
    Installing dependencies using npm
    Running tests using npm
    Bumping version using npm
    Pushing tags
    Creating release draft on GitHub

× Remote history differs. Please pull changes.

Steps to reproduce

1. Have a checkout that does not have all changes from origin and also does not know about them (!) (E.g. by having 2 checkouts of the same repo: Edit, commit and push something in checkout 1, then do the following steps in checkout 2)
2. e.g. np minor
3. Observe failure
4. gitfetch origin
5. Use np minor again, see expected error message

Expected behavior

np should be able to also highlight unpulled changes if the users didn't explicitly git fetch origin recently. I am unsure how, though, as I don't know if just running git fetch origin is ok.

Environment

np - 5.0.2 Node.js - 12.3.1 npm - 6.9.0 Git - 2.21.0.windows.1 OS - Windows 10

---

This issue might be a duplicate of #390 with better explanation of what is happening.

[janpio]

(Interesting side effect: the tags actually got pushed to the remote origin repo, just the commits to master failed. That was unexpected to me, but of course makes sense if you look at the output: The tags are pushed first, and only during the pushing of master does it fail.)

[janpio]

I think this command might be a way to find out if there are remote changes to be fetched/pulled that works without touching the local checkout (we don't want to change anything if the users might not want to):

E:\Projects\Cordova\cordova-plugin-device (master -> origin) ([email protected])
λ git fetch origin --dry-run
From https://github.com/apache/cordova-plugin-device
   594fa61..b888cdc  master     -> origin/master

With --verbose we also see all the stuff that doesn't indicate changes:

E:\Projects\Cordova\cordova-plugin-device (master -> origin) ([email protected])
λ git fetch origin --dry-run --verbose
From https://github.com/apache/cordova-plugin-device
   594fa61..b888cdc  master     -> origin/master
 = [up to date]      1.1.x      -> origin/1.1.x
 = [up to date]      2.0.x      -> origin/2.0.x
 = [up to date]      CB-6127cordova-plugin-device -> origin/CB-6127cordova-plugin-device
 = [up to date]      CB-8438cordova-plugin-device -> origin/CB-8438cordova-plugin-device
 = [up to date]      CB-8600cordova-plugin-device -> origin/CB-8600cordova-plugin-device
 = [up to date]      CB-8860cordova-plugin-device -> origin/CB-8860cordova-plugin-device
 = [up to date]      CB-9128cordova-plugin-device -> origin/CB-9128cordova-plugin-device
 = [up to date]      CB-9381cordova-plugin-device -> origin/CB-9381cordova-plugin-device
 = [up to date]      CB-9444cordova-plugin-device -> origin/CB-9444cordova-plugin-device
 = [up to date]      cdvtest    -> origin/cdvtest
 = [up to date]      dev        -> origin/dev
 = [up to date]      npmpub     -> origin/npmpub
 = [up to date]      old-ID     -> origin/old-ID

So I think we might use git fetch origin --dry-run to output a There seem to be unfetched changes on the remote which might cause a push to fail later. Please fetch changes warning.

[janpio]

Possible implementation, modelled after isRemoteHistoryClean:

exports.isCheckoutFetched = async () => {
    let result;
	try { // Gracefully handle no remote set up.
        result = await execa.stderr('git', ['fetch', 'origin', '--dry-run']);
	} catch (_) {}

	if (result && result !== '') {
		return false;
	}

    return true;
};

And a possible verify method:

exports.verifyCheckoutIsFetched = async () => {
	if (!(await exports.isCheckoutFetched ())) {
		throw new Error('Remote changed not fetched. Please `git fetch` remote changes.');
	}
};

Wording of the message should probably be refined.

[ianwalter]

@janpio I think you should create a PR. It seems like that's the best solution from what I've seen / tried.

But you would need to add the branch name into the command right? Otherwise it would pull other branch/tag updates from origin.

[janpio]

I am not using np any more, also couldn't get other PRs successfully merged as I didn't actually know enough JS to also cover the tests - so I am not going to do this.

But feel free to run with this code and create a PR yourself ❤️

[ianwalter]

I am not using np any more, also couldn't get other PRs successfully merged as I didn't actually know enough JS to also cover the tests - so I am not going to do this.

But feel free to run with this code and create a PR yourself ❤️

Ok, will do, thanks for your help.