OK, I know that the entire point of git is to drive people insane but this is an especially silly problem. clone the repository with git clone https://github.com/OpenSmalltalk/opensmalltalk-vm.git wait... cd opensmalltalk-vm/ ./scripts/updateSCCSVersions -> fatal: Not a git repository: '.git' fatal: unrecognized input What? The annoying part is that it worked (at least, so far as I remember) last time I loaded up a clone of the vm repo. tim -- tim Rowledge; [hidden email]; http://www.rowledge.org/tim The theoreticians have proven that this is unsolvable, but there's three of us, and we're smart... |
Hi Tim,
On Tue, Feb 20, 2018 at 6:29 PM, tim Rowledge <[hidden email]> wrote:
Agreed. It is incumbent upon whoever made the change to fix this. "you break it, you fix it". so please, step up :-) _,,,^..^,,,_ best, Eliot |
Hi Tim & Eliot, On 21 February 2018 at 04:11, Eliot Miranda <[hidden email]> wrote: > > Hi Tim, > > On Tue, Feb 20, 2018 at 6:29 PM, tim Rowledge <[hidden email]> wrote: >> >> >> OK, I know that the entire point of git is to drive people insane but this is an especially silly problem. >> >> clone the repository with >> git clone https://github.com/OpenSmalltalk/opensmalltalk-vm.git >> wait... >> cd opensmalltalk-vm/ >> ./scripts/updateSCCSVersions -> >> fatal: Not a git repository: '.git' >> fatal: unrecognized input >> >> What? >> The annoying part is that it worked (at least, so far as I remember) last time I loaded up a clone of the vm repo. > > > Agreed. It is incumbent upon whoever made the change to fix this. "you break it, you fix it". so please, step up :-) > > _,,,^..^,,,_ > best, Eliot > The offending commit is 0458c6a92651fc8d0bf86158a0407f87470ade50 from 6 Feb. I'm not familiar enough with git to easily fix it, but backing out the change should be straightforward. Cheers, Alistair |
Hi all,
On Wed, Feb 21, 2018 at 8:17 AM Alistair Grant <[hidden email]> wrote:
As part of the transition from SVN to Git, we had to add a script that runs after certain Git commands and inserts a timestamp into the code which is then compiled into the VM as a part of its version string. This approach does not only sound hacky, it actually is hacky, but a Git commit hash was not good enough. Unfortunately, there are many ways to work with Git repositories (e.g. submodules), so Jakob proposed a change that seemed to fix things for some use cases while apparently breaking things for other users. >> The annoying part is that it worked (at least, so far as I remember) last time I loaded up a clone of the vm repo. I've reverted Jakob's change, so please try again. > Thanks for looking into this, Alistair.
|
Hi Jakob, Hi Fabio,
On Tue, Feb 20, 2018 at 11:43 PM, Fabio Niephaus <[hidden email]> wrote:
Thanks. It's good to know what the change is :-). So we're talking about this change, right? : diff --git a/scripts/updateSCCSVersions b/scripts/updateSCCSVersions index 179b008..8593f2f 100755 --- a/scripts/updateSCCSVersions +++ b/scripts/updateSCCSVersions @@ -3,15 +3,13 @@ # platforms/Cross/vm/sqSCCSVersion.h to be checked-in so that its version # info reflects that of the current check-in. -if [ -d `dirname $0`/../.git ]; then - hooks_dir="`git rev-parse --git-dir`/hooks" - mkdir -p $hooks_dir - cp -f $0 $hooks_dir/post-commit - cp -f $0 $hooks_dir/post-merge - cp -f $0 $hooks_dir/post-checkout - cd `dirname $0`/.. +if [ "$(cd "$(dirname $0)" && git rev-parse --is-inside-git-dir)" = false ]; then + hooks_dir="`git rev-parse --git-dir`/hooks" + mkdir -p "$hooks_dir" + cp -f "$0" "$hooks_dir/post-commit" + cp -f "$0" "$hooks_dir/post-merge" + cp -f "$0" "$hooks_dir/post-checkout" else - cd `dirname $0`/../.. if [[ "$(basename $0)" == "post-checkout" ]]; then prevHEAD=$1 newHEAD=$2 Jakob, my understanding is that in shell if one wants to evaluate something enc capture its output one must use backpacks. e.g. using the dirname $0 example above one can say either `dirname $0` or "`dirname $0`", the difference being that the first can expand to multiple tokens but the last will be considered a single then. I don't understand what the intent of "$(cd "$(dirname $0)" && git rev-parse --is-inside-git-dir)" = false is. But in any case if "$(cd "$(dirname $0)" did work the same as "`cd \`dirname $0\` `", it would;t have the desired effect because the command is evaluated in a sub-shell and hence doesn't change the working directory for the git rev-parse --is-inside-git-dir command. I think you need to say something like if (cd `dirname $0`; git rev-parse --is-inside-git-dir); then This evaluates the cd in a sub-shell established by the parentheses, and hence also changes the current directory for the git rev-parse --is-inside-git-dir command. Since "if" takes a command ( /bin/[ is merely a link to /bin/test ) one doesn't need the sub-shell evaluation and can run the git command directly. If we were in the right directory then we could write if git rev-parse --is-inside-git-dir; then The next issue is to get rid of the "fatal: not a git directory" output. You can redirect stderr to /dev/null. hence what I *think* you want to write is if (cd `dirname $0`; git rev-parse --is-inside-git-dir 2>/dev/null); then HTH And Jakob, I hope you're not offended! The request to revert was not in any way a criticism of you. We all make mistakes and often those who make most mistakes are contributing the most.
_,,,^..^,,,_ best, Eliot |
Hi Eliot, no worries, I am not offended. As I wrote in the PR: "pick those parts of the changes that you like". Sorry that it caused trouble, which is not very likable, of course. $() is the POSIX syntax for command substitution and has the same semantics as the backticks. I find it more readable than the backticks, especially when combined with regular quotes. Also, it simplifies quoting and nesting (which I did here). I agree that the comparison with "false" might look like someone did not know their way around shell programming and return codes in particular, but rev-parse --is-inside-git-dir really does print either "true" or "false" to stdout. ;-) And it exists with 0 in either case. While the command substitution runs a subshell, the cd and git rev-parse should already run in the same one and they are combined with &&. Rather than muting the "not a git repository" message, I would like to understand why it occurs in the first place. But I have not yet got around to investigate it myself. In another thread, I proposed to add a "set -x" at the top of the script to see where the message occurs; it might also be one of the commands in the changed lines further down the script. I had hoped that somebody would find the time earlier than I would. Kind regards, Jakob 2018-02-21 20:03 GMT+01:00 Eliot Miranda <[hidden email]>: > > Hi Jakob, Hi Fabio, > > On Tue, Feb 20, 2018 at 11:43 PM, Fabio Niephaus <[hidden email]> wrote: >> >> >> Hi all, >> >> On Wed, Feb 21, 2018 at 8:17 AM Alistair Grant <[hidden email]> wrote: >>> >>> >>> Hi Tim & Eliot, >>> >>> On 21 February 2018 at 04:11, Eliot Miranda <[hidden email]> wrote: >>> > >>> > Hi Tim, >>> > >>> > On Tue, Feb 20, 2018 at 6:29 PM, tim Rowledge <[hidden email]> wrote: >>> >> >>> >> >>> >> OK, I know that the entire point of git is to drive people insane but this is an especially silly problem. >>> >> >>> >> clone the repository with >>> >> git clone https://github.com/OpenSmalltalk/opensmalltalk-vm.git >>> >> wait... >>> >> cd opensmalltalk-vm/ >>> >> ./scripts/updateSCCSVersions -> >>> >> fatal: Not a git repository: '.git' >>> >> fatal: unrecognized input >>> >> >>> >> What? >> >> >> As part of the transition from SVN to Git, we had to add a script that runs after certain Git commands and inserts a timestamp into the code which is then compiled into the VM as a part of its version string. This approach does not only sound hacky, it actually is hacky, but a Git commit hash was not good enough. Unfortunately, there are many ways to work with Git repositories (e.g. submodules), so Jakob proposed a change that seemed to fix things for some use cases while apparently breaking things for other users. >> >>> >>> >> The annoying part is that it worked (at least, so far as I remember) last time I loaded up a clone of the vm repo. >> >> >> I've reverted Jakob's change, so please try again. > > > Thanks. It's good to know what the change is :-). > > So we're talking about this change, right? : > > diff --git a/scripts/updateSCCSVersions b/scripts/updateSCCSVersions > index 179b008..8593f2f 100755 > --- a/scripts/updateSCCSVersions > +++ b/scripts/updateSCCSVersions > @@ -3,15 +3,13 @@ > # platforms/Cross/vm/sqSCCSVersion.h to be checked-in so that its version > # info reflects that of the current check-in. > > -if [ -d `dirname $0`/../.git ]; then > - hooks_dir="`git rev-parse --git-dir`/hooks" > - mkdir -p $hooks_dir > - cp -f $0 $hooks_dir/post-commit > - cp -f $0 $hooks_dir/post-merge > - cp -f $0 $hooks_dir/post-checkout > - cd `dirname $0`/.. > +if [ "$(cd "$(dirname $0)" && git rev-parse --is-inside-git-dir)" = false ]; then > + hooks_dir="`git rev-parse --git-dir`/hooks" > + mkdir -p "$hooks_dir" > + cp -f "$0" "$hooks_dir/post-commit" > + cp -f "$0" "$hooks_dir/post-merge" > + cp -f "$0" "$hooks_dir/post-checkout" > else > - cd `dirname $0`/../.. > if [[ "$(basename $0)" == "post-checkout" ]]; then > prevHEAD=$1 > newHEAD=$2 > > Jakob, my understanding is that in shell if one wants to evaluate something enc capture its output one must use backpacks. e.g. using the dirname $0 example above one can say either `dirname $0` or "`dirname $0`", the difference being that the first can expand to multiple tokens but the last will be considered a single then. I don't understand what the intent of "$(cd "$(dirname $0)" && git rev-parse --is-inside-git-dir)" = false is. But in any case if "$(cd "$(dirname $0)" did work the same as "`cd \`dirname $0\` `", it would;t have the desired effect because the command is evaluated in a sub-shell and hence doesn't change the working directory for the git rev-parse --is-inside-git-dir command. I think you need to say something like > > if (cd `dirname $0`; git rev-parse --is-inside-git-dir); then > > This evaluates the cd in a sub-shell established by the parentheses, and hence also changes the current directory for the git rev-parse --is-inside-git-dir command. Since "if" takes a command ( /bin/[ is merely a link to /bin/test ) one doesn't need the sub-shell evaluation and can run the git command directly. If we were in the right directory then we could write > > if git rev-parse --is-inside-git-dir; then > > The next issue is to get rid of the "fatal: not a git directory" output. You can redirect stderr to /dev/null. hence what I *think* you want to write is > > if (cd `dirname $0`; git rev-parse --is-inside-git-dir 2>/dev/null); then > > HTH > > And Jakob, I hope you're not offended! The request to revert was not in any way a criticism of you. We all make mistakes and often those who make most mistakes are contributing the most. > > >>> > Agreed. It is incumbent upon whoever made the change to fix this. "you break it, you fix it". so please, step up :-) >>> > >>> > _,,,^..^,,,_ >>> > best, Eliot >>> > >>> >>> The offending commit is 0458c6a92651fc8d0bf86158a0407f87470ade50 from >>> 6 Feb. I'm not familiar enough with git to easily fix it, but backing >>> out the change should be straightforward. >> >> >> Thanks for looking into this, Alistair. >> >>> >>> >>> Cheers, >>> Alistair > > > > _,,,^..^,,,_ > best, Eliot > |
Free forum by Nabble | Edit this page |