pharo-project / pharo

Pharo is a dynamic reflective pure object-oriented language supporting live programming inspired by Smalltalk.
http://pharo.org
Other
1.2k stars 355 forks source link

The system version hash does not have the first character #2379

Closed pavel-krivanek closed 5 years ago

pavel-krivanek commented 5 years ago

The commit has for Pharo 8 build 33 is a896d6da6b2617aa96aed6d3614402d518d8da4 while the real git commit hash is ca896d6da6b2617aa96aed6d3614402d518d8da4. The first cheracter is missing.

pavel-krivanek commented 5 years ago

The Pharo 7.0.1 has correct commit hash

akgrant43 commented 5 years ago

Hi Pavel,

I've just been looking at the same thing. Do you have a solution (It looks like the SystemVersion commitHash is being set incorrectly, but that's as far as I've got)?

Cheers, Alistair

akgrant43 commented 5 years ago

P.S.: I was tracing backwards from the sources file name being incorrect during bootstrap.

pavel-krivanek commented 5 years ago

it is not related to your changes because the the commit e375d69ce93821ad965a007f897965c893462013 has already this issue

akgrant43 commented 5 years ago

Yep. I was just looking at it because it means that the Iceberg doesn't work until the correct commit has been checked out.

pavel-krivanek commented 5 years ago

It's very probably caused by this commit: https://github.com/pharo-project/pharo/commit/942cb0da24c1dce47aedeea3a7666bee59214a50#diff-726cefa75021642b162d5baa3be9c10f

pavel-krivanek commented 5 years ago

@guillep can you look at it?

PharoProject commented 5 years ago

Really ? i've made that commit 4 months ago with tests even, and the build just broke now... That's 100% something else :)

Le lun. 28 janv. 2019 à 17:12, Pavel Krivanek notifications@github.com a écrit :

@guillep https://github.com/guillep can you look at it?

You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/pharo-project/pharo/issues/2379#issuecomment-458194122, or mute the thread https://github.com/notifications/unsubscribe-auth/AP9ZTGwz3nJs3XI-0PpV8Tlu_brLF4Ilks5vHyFZgaJpZM4aWMrm .

--

Guille Polito

Research Engineer

Centre de Recherche en Informatique, Signal et Automatique de Lille

CRIStAL - UMR 9189

French National Center for Scientific Research - http://www.cnrs.fr http://www.cnrs.fr

Web: http://guillep.github.io http://guillep.github.io

Phone: +33 06 52 70 66 13

guillep commented 5 years ago

I mean, first we should find the commit that introduced the problem because this was working. The changes I did 4 months ago were to support versions coming from git-describe. git-describe sends an extra "g" character before the commit hash.

So, maybe somehow we are not using git describe anymore, or somebody is pre-filtering the extra "g" before it arrives to pharo

guillep commented 5 years ago

It seems this happened between builds 2 and 3 of pharo8, the first commit where this is broken (build 3) is this one:

https://github.com/pharo-project/pharo/commit/18299527942471f3a94108e1c50ac19eb2349383

guillep commented 5 years ago

So it seems that now we are creating the version info by hand, and not respecting the git-describe-like version info that we were expecting...

pavel-krivanek commented 5 years ago

then, should we remove the g-prefix removing code?

guillep commented 5 years ago

Maybe, I don't know :) Probably we should add a new method in PBVersionInfo that knows how to read our ad-hoc version info.

fromVersionInfoString: aString 
    "Expects a string with the form:
            [SemanticVersion]-numberOfCommits-[commitish]"
    | parts commitish semanticVersion |
    parts := aString substrings: '-'.
    commitish := parts last.
    semanticVersion := (parts allButLast: 2) joinUsing: '-'.

    ^ self basicNew
        fromSemanticVersion: semanticVersion;
        commitHash: commitish;
        yourself

But to me it's not clear yet unless I test it. Because git describe will provide something in the form v1.5.3-21-g975b and I'm not sure that we are respecting that either...