Re: Git 2 force commits but Git 1 doesn't
- From
brian m. carlson <sandals@crustytoothpaste.net>
- Date
- Jun 23, 2020, 15:30 UTC
- Message-ID
- <20200623151951.GS6531@camp.crustytoothpaste.net>
- In-Reply-To
- <09a7fa54-d7ae-772c-fb36-29dbd27bc626@web.de>
On 2020-06-23 at 08:59:28, René Scharfe wrote:
> The following patch helps by avoiding a commit flag collision between > http-push.c and commit-reach.c. I don't know if it causes other > collisions, though.
Nice catch.
I don't think it collides with other things because the other things that overlap are either builtin code (which isn't used in git http-push) or upload-pack.c, which would already be having problems before this because it overlaps both before and after (and doesn't appear to be used in http-push either). Therefore, I feel confident in saying this probably doesn't make anything worse.
> How could we possibly check that? Perhaps by having a commit flag > register (a global unsigned int) and having functions announce their > bits in it. Colliding announcements would BUG().
By my count, we have only 88 individual bits used. If we moved all of the builtin functions plus upload-pack (which shouldn't overlap) to a single range, that would account for 53 bits, which would leave us 35 bits for all the rest of the code. Since we need at most 27 bits for a builtin command, if we used a 64-bit integer, we'd have space for all the remaining bits not to overlap, plus bits for the type and flags bits.
Since we'd be doing only bit operations on the flags variable, the performance impact on 32-bit systems would be very minimal, although we'd allocate an extra 4 bytes for struct object. I don't know if that's a problem.
Assuming we don't want to do that right now, may I have your sign-off for the following code, René, so I can add it to a patch along with my test?
Show 33 quoted lines
> diff --git a/http-push.c b/http-push.c
> index 822f326599..99adbebdcf 100644
> --- a/http-push.c
> +++ b/http-push.c
> @@ -70,10 +70,10 @@ enum XML_Status {
> #define LOCK_REFRESH 30
>
> /* Remember to update object flag allocation in object.h */
> -#define LOCAL (1u<<16)
> -#define REMOTE (1u<<17)
> -#define FETCHING (1u<<18)
> -#define PUSHING (1u<<19)
> +#define LOCAL (1u<<11)
> +#define REMOTE (1u<<12)
> +#define FETCHING (1u<<13)
> +#define PUSHING (1u<<14)
>
> /* We allow "recursive" symbolic refs. Only within reason, though */
> #define MAXDEPTH 5
> diff --git a/object.h b/object.h
> index b22328b838..a496d2e4e1 100644
> --- a/object.h
> +++ b/object.h
> @@ -67,7 +67,7 @@ struct object_array {
> * builtin/blame.c: 12-13
> * bisect.c: 16
> * bundle.c: 16
> - * http-push.c: 16-----19
> + * http-push.c: 11-----14
> * commit-graph.c: 15
> * commit-reach.c: 16-----19
> * sha1-name.c: 20
> -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204