Skip to content

Conversation

gklyne
Copy link
Contributor

@gklyne gklyne commented Feb 22, 2019

See #1110, #1111

This fix raises an error when the version information returned by running git is empty, thereby falling back to using package.json. (This is a new attempt to submit previous PR #1111, but this time based upon and updating the release/v5.0.0 branch.)

@kjetilk kjetilk requested a review from megoth March 7, 2019 11:08
Copy link
Contributor

@megoth megoth left a comment

Choose a reason for hiding this comment

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

LGTM

@kjetilk kjetilk merged commit 5cbfb79 into nodeSolidServer:release/v5.0.0 Mar 7, 2019
@linonetwo
Copy link
Contributor

linonetwo commented Jun 17, 2019

Why it is showing Solid server (v0.33.8)?

I've installed latest v5.1.5 , is this a bug or a feature?

@gklyne
Copy link
Contributor Author

gklyne commented Jun 18, 2019

@linonetwo I'd check to see what git describe --tags is returning when run from the bin/cli directory. (My fix should only affect installs that are not from a local git repo.)

@linonetwo
Copy link
Contributor

I've install solid via npm i -g.

After install, it shows 5.1.5 installed, but then I run solid -V shows Solid server (v0.33.8)

@kjetilk
Copy link
Member

kjetilk commented Jun 19, 2019

strange... I can't reproduce, and I don't know what could produce such a result. What does
which solid say? Could it be something else entirely?

@gklyne
Copy link
Contributor Author

gklyne commented Jun 19, 2019

Is it possible that the npm install is inside some other git repository?

Running git status, or git describe --tags, in the directory with the installed software (i.e. the directory from which solid) might quickly eliminate this possibility.

@linonetwo
Copy link
Contributor

linonetwo commented Jun 19, 2019

It is not a git repo:

~$ which solid
~/.nvm/versions/node/v12.4.0/bin/solid
~$ solid -V
v0.33.8
~$ cd ~/.nvm/versions/node/v12.4.0/bin && git status
Not currently on any branch.
nothing to commit, working directory clean
~/.nvm/versions/node/v12.4.0/bin$

@gklyne
Copy link
Contributor Author

gklyne commented Jun 19, 2019

Thanks - I agree, it's not that.

Next places I'd look are:

.../bin/lib/cli.js, line 33:

  } catch (e) {
    // Obtain version from package.json
    const { version } = require(path.join(__dirname, '../../package.json'))
    return version
  }

.../package.json, line 4:

  "version": "5.1.5",

Beyond that, as far as I can tell, the version display is handled by the commander command line parser.

I'd be tempted to try inserting a console.log(version) in cli.js before the return version to (a) confirm it's taking that code path, and (b) check the value of version that comes from package.json. If it's correct the then it looks as if going astray in the handoff to commander at line 11: program.version(getVersion())

@linonetwo linonetwo mentioned this pull request Jul 3, 2019
@linonetwo
Copy link
Contributor

linonetwo commented Jul 3, 2019

Oh, note that, even if git status returns Not currently on any branch.,

git describe --tags can still return something:

xxx@xxx-ubuntu:~/.nvm/versions/node/v12.4.0/bin$ git describe --tags
v0.33.8

xxx@xxx-ubuntu:~/.nvm/versions/node/v12.4.0/bin$ git status
Not currently on any branch.
nothing to commit, working directory clean

xxx@xxx-ubuntu:~/.nvm/versions/node/v12.4.0/bin$ cat ../../package.json
cat: ../../package.json: No such file or directory

linonetwo added a commit to linonetwo/node-solid-server that referenced this pull request Jul 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants