Skip to content
This repository has been archived by the owner. It is now read-only.

embed git sha in code to detect whether code is built from current resources #2625

Closed
shane-tomlinson opened this issue Jun 21, 2015 · 9 comments
Closed
Assignees

Comments

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Jun 21, 2015

We have /ver.json that reports the current git sha, but I am never 100% confident the code was built from that sha. If the git sha were embedded in the code, or as part of the filename, I'd have a lot more confidence the current code is what we think it is.

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Jun 21, 2015

@shane-tomlinson should that be available on some global namespace on window?
Filename can be tricky due to caching thing we have, but is also possible.

@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented Jun 21, 2015

@vladikoff - or perhaps even a comment at the top of the file.

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Jun 29, 2015

Could be done as part of #2626

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Aug 6, 2015

from irc:

the fxa-content .git repo is not deployed
so that sha is already available in ver.json.
the rpm build script puts it there
https://github.com/mozilla/fxa-content-server/blob/master/server/lib/routes/get-ver.json.js#L10 or https://github.com/mozilla/fxa-content-server/blob/master/server/lib/version.js#L35
get-ver.json could use a little cleanup. It requires version.json at startup . carefully requires it later inside a try/catch
I guess the thing is that we just package and ship whatever is left after grunt build. .git is not present at that point, but we could change order so config/version.json is available before grunt.

@jrgm
Copy link
Contributor

@jrgm jrgm commented Aug 6, 2015

@shane-tomlinson - where are you not confident that the version in /ver.json does not represent the sha of the code that built it? Prod/Stage? latest? other?

@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented Aug 10, 2015

@shane-tomlinson - where are you not confident that the version in /ver.json does not represent the sha of the code that built it? Prod/Stage? latest? other?

All of the above. Embedding the git sha in the code is just another way to verify expectations match reality.

@rfk rfk added this to the FxA-0: quality and maintenance milestone Aug 21, 2015
shane-tomlinson pushed a commit that referenced this issue Oct 12, 2015
Output looks like:
```js
/*! fxa-content-server@0.47.1 -- Mon Oct 12 2015 20:04:55
 *
 * Git sha: 22cfb27e645042d1eb70e8509b2a36f8235837ff
 *
 * This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/.
 *
 * For more information, see https://github.com/mozilla/fxa-content-server/
 */
```

fixes #2625
@shane-tomlinson shane-tomlinson removed their assignment Oct 19, 2015
@vladikoff vladikoff self-assigned this Oct 20, 2015
@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Oct 21, 2015

blocked by changes to an external script #3160 (comment)

shane-tomlinson pushed a commit that referenced this issue Oct 21, 2015
Output looks like:
```js
/*! fxa-content-server@0.47.1 -- Mon Oct 12 2015 20:04:55
 *
 * Git sha: 22cfb27e645042d1eb70e8509b2a36f8235837ff
 *
 * This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/.
 *
 * For more information, see https://github.com/mozilla/fxa-content-server/
 */
```

fixes #2625
@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Oct 26, 2015

added waffle:review and removed waffle:blocked labels 8 hours ago

Hm, Not sure what happened there, did this move automatically? AFAIK the issue / pr is blocked by an external script.

@rfk
Copy link
Member

@rfk rfk commented Oct 26, 2015

from meeting, @jrgm said this was ready to merge

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants