Fit the building tools to Plan 9 environment #694
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lufia
force-pushed
the
compat-p9
branch
13 times, most recently
from
cdd5d7b to
4ebd56a
Compare
|
/submit |
|
Submitted as pull.694.git.1596675905.gitgitgadget@gmail.com |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
GIT-VERSION-GEN
Outdated
| @@ -26,7 +26,7 @@ else | |||
| VN="$DEF_VER" | |||
There was a problem hiding this comment.
On the Git mailing list, "brian m. carlson" wrote (reply to this):
--THYEXwetZJOK3OLY
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On 2020-08-06 at 01:05:03, lufia via GitGitGadget wrote:
> From: lufia <lufia@lufia.org>
>=20
> That haven't any commands: cut, expr and printf.
Is this ANSI/POSIX environment the one mentioned at [0]? That page
describes it as supporting POSIX 1003.1-1990, which is a bit dated. I
think we generally assume one has the 2001 edition or later, so you'll
have your work cut out for you.
> And its sed(1)'s label is limited to maximum seven characters.
> Therefore I replaced some labels to drop a character.
>=20
> * close -> cl
> * continue -> cont (cnt is used for count)
> * line -> ln
> * hered -> hdoc
> * shell -> sh
> * string -> str
>=20
> Signed-off-by: lufia <lufia@lufia.org>
I will note that usually the project prefers to have a human's personal
name here and in the commit metadata instead of a username. Junio may
chime in here with an opinion.
> command_list () {
> - eval "grep -ve '^#' $exclude_programs" <"$1"
> + eval "grep -v -e '^#' $exclude_programs" <"$1"
Is it really the case that Plan 9's grep cannot deal with bundled short
options? That seems to be a significant departure from POSIX and Unix
behavior. Regardless, this should be explained in the commit message.
> get_categories () {
> - tr ' ' '\n'|
> + tr ' ' '\012'|
Okay, I guess. Is this something we need to handle elsewhere as well?
The commit message should tell us why this is necessary, and what Plan 9
does and doesn't support.
> grep -v '^$' |
> sort |
> uniq
> @@ -18,13 +18,13 @@ get_categories () {
> =20
> category_list () {
> command_list "$1" |
> - cut -c 40- |
> + awk '{ print substr($0, 40) }' |
I can tell that you haven't gotten the test suite working because I've
added a large number of cut invocations there. I suspect you're going
to need to provide a portability wrapper there that implements it using
awk, sed, or perl.
> +if test -z "$(echo -n)"
> +then
> + alias print=3D'echo -n'
> +else
> + alias print=3D'printf %s'
> +fi
Let's avoid an alias here (especially with a common builtin name) and
instead use a shell function. Maybe like this (not tab-indented):
print_nonl () {
if command -v printf >/dev/null 2>&1
then
printf "%s" "$@"
else
echo -n "$@"
fi
}
Notice also that we prefer the standard form and fall back to the
nonstandard form if the system is less capable. I don't know if Plan 9
supports "command -v"; "type" may be preferable, but isn't supported by
some other shells (e.g., posh). For portability reasons, we may need to
try to run printf and see if it fails.
This is also going to need some patching in the testsuite, since we use
printf extensively (more than 1300 times). I do hope you have perl
available.
[0] http://doc.cat-v.org/plan_9/4th_edition/papers/ape
--=20
brian m. carlson: Houston, Texas, US
--THYEXwetZJOK3OLY
Content-Type: application/pgp-signature; name="signature.asc"
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.2.20 (GNU/Linux)
iHUEABYKAB0WIQQILOaKnbxl+4PRw5F8DEliiIeigQUCXytlFwAKCRB8DEliiIei
gTw1AP9fJWCz9h/8SS9v0zL6JOT8Au4nANBpuEcV6sYrMX6jIgEAyfeYBcaBKj2r
cADIRG9C36oCDBXa+5BRsyzPMi9KIw4=
=F+L7
-----END PGP SIGNATURE-----
--THYEXwetZJOK3OLY--
There was a problem hiding this comment.
On the Git mailing list, Kyohei Kadota wrote (reply to this):
> On 2020-08-06 at 01:05:03, lufia via GitGitGadget wrote:
> > From: lufia <lufia@lufia.org>
> >
> > That haven't any commands: cut, expr and printf.
>
> Is this ANSI/POSIX environment the one mentioned at [0]? That page
> describes it as supporting POSIX 1003.1-1990, which is a bit dated. I
> think we generally assume one has the 2001 edition or later, so you'll
> have your work cut out for you.
Yes, the layer I told is APE.
I guess originally APE might be introduced for porting Ghostscript to Plan 9.
> > And its sed(1)'s label is limited to maximum seven characters.
> > Therefore I replaced some labels to drop a character.
> >
> > * close -> cl
> > * continue -> cont (cnt is used for count)
> > * line -> ln
> > * hered -> hdoc
> > * shell -> sh
> > * string -> str
> >
> > Signed-off-by: lufia <lufia@lufia.org>
>
> I will note that usually the project prefers to have a human's personal
> name here and in the commit metadata instead of a username. Junio may
> chime in here with an opinion.
I see. I will rename them.
> > command_list () {
> > - eval "grep -ve '^#' $exclude_programs" <"$1"
> > + eval "grep -v -e '^#' $exclude_programs" <"$1"
>
> Is it really the case that Plan 9's grep cannot deal with bundled short
> options? That seems to be a significant departure from POSIX and Unix
> behavior. Regardless, this should be explained in the commit message.
This is awful.
But now, APE's grep (/bin/ape/grep) is a simple wrapper for native
grep (/bin/grep),
its option parser is a very rough implementation.
https://github.com/0intro/plan9-contrib/blob/master/rc/bin/ape/grep
> > get_categories () {
> > - tr ' ' '\n'|
> > + tr ' ' '\012'|
>
> Okay, I guess. Is this something we need to handle elsewhere as well?
> The commit message should tell us why this is necessary, and what Plan 9
> does and doesn't support.
Yeah. I will edit the message.
Plan 9's tr(1) handles only \(16 bit octal) and \x(16 bit hexadecimal)
escape sequences.
If another character after leading backslash, tr(1) will replace \c to c.
> > grep -v '^$' |
> > sort |
> > uniq
> > @@ -18,13 +18,13 @@ get_categories () {
> >
> > category_list () {
> > command_list "$1" |
> > - cut -c 40- |
> > + awk '{ print substr($0, 40) }' |
>
> I can tell that you haven't gotten the test suite working because I've
> added a large number of cut invocations there. I suspect you're going
> to need to provide a portability wrapper there that implements it using
> awk, sed, or perl.
I see. If I'd like to put those wrappers to the repository, is there
the best place for them?
> > +if test -z "$(echo -n)"
> > +then
> > + alias print='echo -n'
> > +else
> > + alias print='printf %s'
> > +fi
>
> Let's avoid an alias here (especially with a common builtin name) and
> instead use a shell function. Maybe like this (not tab-indented):
>
> print_nonl () {
> if command -v printf >/dev/null 2>&1
> then
> printf "%s" "$@"
> else
> echo -n "$@"
> fi
> }
>
> Notice also that we prefer the standard form and fall back to the
> nonstandard form if the system is less capable. I don't know if Plan 9
> supports "command -v"; "type" may be preferable, but isn't supported by
> some other shells (e.g., posh). For portability reasons, we may need to
> try to run printf and see if it fails.
>
> This is also going to need some patching in the testsuite, since we use
> printf extensively (more than 1300 times). I do hope you have perl
> available.
In fact, Plan 9's ape/sh is pdksh, so it supports "command -v".
However I think, like the above comment, it might be better to create
the printf(1) wrapper.
---
kadota
> [0] http://doc.cat-v.org/plan_9/4th_edition/papers/ape
> --
> brian m. carlson: Houston, Texas, US
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> I will note that usually the project prefers to have a human's personal
> name here and in the commit metadata instead of a username. Junio may
> chime in here with an opinion.
Thanks, I just did.
>> command_list () {
>> - eval "grep -ve '^#' $exclude_programs" <"$1"
>> + eval "grep -v -e '^#' $exclude_programs" <"$1"
>
> Is it really the case that Plan 9's grep cannot deal with bundled short
> options? That seems to be a significant departure from POSIX and Unix
> behavior. Regardless, this should be explained in the commit message.
I am not interested in this ball of wax, but there are some pieces
that are worth salvaging. This is one of those good bits.
diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 9db2f4feab..a7cc01caf9 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -26,7 +26,7 @@ else
VN="$DEF_VER"
fi
-VN=$(expr "$VN" : v*'\(.*\)')
+VN=${VN#v}
if test -r $GVF
then
BUT.
Dealing with "grep" that cannot take "-ve" and forces developers to
spell it as "-v -e" is not one of them. So is forbidding use of
"cut".
>> get_categories () {
>> - tr ' ' '\n'|
>> + tr ' ' '\012'|
>
> Okay, I guess. Is this something we need to handle elsewhere as well?
> The commit message should tell us why this is necessary, and what Plan 9
> does and doesn't support.
Yeah, POSIX does want you to understand '\n' and others, but this is
within reason for portability that I think we could swallow.
>> +if test -z "$(echo -n)"
>> +then
>> + alias print='echo -n'
>> +else
>> + alias print='printf %s'
>> +fi
>
> Let's avoid an alias here (especially with a common builtin name) and
> instead use a shell function. Maybe like this (not tab-indented):
>
> print_nonl () {
> if command -v printf >/dev/null 2>&1
> then
> printf "%s" "$@"
> else
> echo -n "$@"
> fi
> }
I'd rather not to see this done; "echo -n" and "echo '...\c'" are
not portable and we do want people to use 'printf'. This directly
goes against it.
> This is also going to need some patching in the testsuite, since we use
> printf extensively (more than 1300 times). I do hope you have perl
> available.
Eh, so what would Plan9 people do with Perl? Write a single-liner
'printf' script and put it in a directory on their $PATH?
I am not sure if it is worth crippling the toolset our developers
are allowed to choose from and use in our scripts and tests like
this patch tries to do.
If this were Windows, it might have been a different story, but what
we are talking about is Plan 9, which had the last "fourth edition"
in 2002, right? During the 18 years since then, none of its users
and developers work on porting many OSS packages, whose primary user
base are on POSIXy systems, to it and we need to apply patches like
these to each and every OSS packages of interest? I would have
expected that any exotic-minority-but-thriving-platform would be
able to tell its users "here are ports of POSIXy programs---install
them and it can become usable by those who only know Linux"?
So, I dunno.
There was a problem hiding this comment.
On the Git mailing list, "brian m. carlson" wrote (reply to this):
--hHWLQfXTYDoKhP50
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On 2020-08-06 at 13:49:43, Kyohei Kadota wrote:
> I see. If I'd like to put those wrappers to the repository, is there
> the best place for them?
If you can stuff them in a shell function, then I'd just put those in a
file like t/test-lib-wrappers.sh and conditionally source them from
t/test-lib.sh. Otherwise, I'd create a subdirectory of t and then add
that to the PATH if necessary.
If you're going to use it outside of the testsuite, then maybe
compat/scripts might be a good idea.
> In fact, Plan 9's ape/sh is pdksh, so it supports "command -v".
> However I think, like the above comment, it might be better to create
> the printf(1) wrapper.
Awesome. I haven't used pdksh in a long time, but if it supports that,
all the better. I'm surprised it doesn't have printf as a builtin, though.
--=20
brian m. carlson: Houston, Texas, US
--hHWLQfXTYDoKhP50
Content-Type: application/pgp-signature; name="signature.asc"
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.2.20 (GNU/Linux)
iHUEABYKAB0WIQQILOaKnbxl+4PRw5F8DEliiIeigQUCXyyXiwAKCRB8DEliiIei
gQqAAP9xt0cAxXTUmOlCSLmAcAqd4CwsDxH5/Sg+yi8zzlGaSQEAwilgZ8uKiGbG
LtJ06s4v5918YhI6M1kRMs7k7zjIUAc=
=KUog
-----END PGP SIGNATURE-----
--hHWLQfXTYDoKhP50--
There was a problem hiding this comment.
On the Git mailing list, Eric Sunshine wrote (reply to this):
On Thu, Aug 6, 2020 at 7:52 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> On 2020-08-06 at 13:49:43, Kyohei Kadota wrote:
> > I see. If I'd like to put those wrappers to the repository, is there
> > the best place for them?
>
> If you can stuff them in a shell function, then I'd just put those in a
> file like t/test-lib-wrappers.sh and conditionally source them from
> t/test-lib.sh.
There are already platform-specific shell functions in test-lib.sh[1]
and some larger compatibility functions in test-lib-functions.sh[2],
so those might be better locations for Plan9 compatibility wrappers
than creating a new file (or not?).
[1]: see the section commented:
"Fix some commands on Windows, and other OS-specific things"
[2]: see, for instance, the mingw_test_cmp() function
There was a problem hiding this comment.
On the Git mailing list, Kyohei Kadota wrote (reply to this):
>
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
> > I will note that usually the project prefers to have a human's personal
> > name here and in the commit metadata instead of a username. Junio may
> > chime in here with an opinion.
>
> Thanks, I just did.
>
> >> command_list () {
> >> - eval "grep -ve '^#' $exclude_programs" <"$1"
> >> + eval "grep -v -e '^#' $exclude_programs" <"$1"
> >
> > Is it really the case that Plan 9's grep cannot deal with bundled short
> > options? That seems to be a significant departure from POSIX and Unix
> > behavior. Regardless, this should be explained in the commit message.
>
> I am not interested in this ball of wax, but there are some pieces
> that are worth salvaging. This is one of those good bits.
>
> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
> index 9db2f4feab..a7cc01caf9 100755
> --- a/GIT-VERSION-GEN
> +++ b/GIT-VERSION-GEN
> @@ -26,7 +26,7 @@ else
> VN="$DEF_VER"
> fi
>
> -VN=$(expr "$VN" : v*'\(.*\)')
> +VN=${VN#v}
>
> if test -r $GVF
> then
>
> BUT.
>
> Dealing with "grep" that cannot take "-ve" and forces developers to
> spell it as "-v -e" is not one of them. So is forbidding use of
> "cut".
I see. I will rewrite Plan 9's ape/grep to handle bundled options well.
> >> get_categories () {
> >> - tr ' ' '\n'|
> >> + tr ' ' '\012'|
> >
> > Okay, I guess. Is this something we need to handle elsewhere as well?
> > The commit message should tell us why this is necessary, and what Plan 9
> > does and doesn't support.
>
> Yeah, POSIX does want you to understand '\n' and others, but this is
> within reason for portability that I think we could swallow.
I'm happy, thanks.
> >> +if test -z "$(echo -n)"
> >> +then
> >> + alias print='echo -n'
> >> +else
> >> + alias print='printf %s'
> >> +fi
> >
> > Let's avoid an alias here (especially with a common builtin name) and
> > instead use a shell function. Maybe like this (not tab-indented):
> >
> > print_nonl () {
> > if command -v printf >/dev/null 2>&1
> > then
> > printf "%s" "$@"
> > else
> > echo -n "$@"
> > fi
> > }
>
> I'd rather not to see this done; "echo -n" and "echo '...\c'" are
> not portable and we do want people to use 'printf'. This directly
> goes against it.
>
> > This is also going to need some patching in the testsuite, since we use
> > printf extensively (more than 1300 times). I do hope you have perl
> > available.
>
> Eh, so what would Plan9 people do with Perl? Write a single-liner
> 'printf' script and put it in a directory on their $PATH?
>
> I am not sure if it is worth crippling the toolset our developers
> are allowed to choose from and use in our scripts and tests like
> this patch tries to do.
Like other topics, I decide to implement printf(1).
These new POSIX tools will be installed to Plan 9 systems
from other repo or patches before building git.
> If this were Windows, it might have been a different story, but what
> we are talking about is Plan 9, which had the last "fourth edition"
> in 2002, right? During the 18 years since then, none of its users
> and developers work on porting many OSS packages, whose primary user
> base are on POSIXy systems, to it and we need to apply patches like
> these to each and every OSS packages of interest? I would have
> expected that any exotic-minority-but-thriving-platform would be
> able to tell its users "here are ports of POSIXy programs---install
> them and it can become usable by those who only know Linux"?
>
> So, I dunno.
Final official distribution from Bell labs was released in Jan 2015.
Now, There are some distributions that forked from official Plan 9 and
are maintained by that community,
such as 9legacy, 9front or others.
I will try to implement POSIX toolsets if it is needed because POSIX
is a large spec.
Thanks for your advice.
config.mak.uname
Outdated
| @@ -4,12 +4,12 @@ | |||
| # Microsoft's Safe Exception Handling in libraries (such as zlib). | |||
There was a problem hiding this comment.
On the Git mailing list, "brian m. carlson" wrote (reply to this):
--OxDG9cJJSSQMUzGF
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On 2020-08-06 at 01:05:01, lufia via GitGitGadget wrote:
> From: lufia <lufia@lufia.org>
>=20
> In the not POSIX environment, like Plan 9, sh might not be looked up
> in the directories named by the $PATH.
I think Git's editor handling assumes that sh is somewhere in the PATH,
so it might be fine for us to just ask the user to adjust PATH
appropriately before running make. I don't have a strong preference; if
this works on a standard Unix machine, which it looks like it should
(although I haven't tested), I'm fine with it.
--=20
brian m. carlson: Houston, Texas, US
--OxDG9cJJSSQMUzGF
Content-Type: application/pgp-signature; name="signature.asc"
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.2.20 (GNU/Linux)
iHUEABYKAB0WIQQILOaKnbxl+4PRw5F8DEliiIeigQUCXytnYgAKCRB8DEliiIei
ga49AQCdGtTEM77h+OMNj2beRuo2OL78RWgq3+wi27Y5qrkCYgD8DCQtGgTwpXYv
P1d3A9WgpnivXBBh7FrE0GeXs6oqjwk=
=XajD
-----END PGP SIGNATURE-----
--OxDG9cJJSSQMUzGF--
There was a problem hiding this comment.
On the Git mailing list, Eric Sunshine wrote (reply to this):
On Wed, Aug 5, 2020 at 10:14 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> On 2020-08-06 at 01:05:01, lufia via GitGitGadget wrote:
> > In the not POSIX environment, like Plan 9, sh might not be looked up
> > in the directories named by the $PATH.
>
> I think Git's editor handling assumes that sh is somewhere in the PATH,
> so it might be fine for us to just ask the user to adjust PATH
> appropriately before running make. I don't have a strong preference; if
> this works on a standard Unix machine, which it looks like it should
> (although I haven't tested), I'm fine with it.
This does, however, have a bit of a chicken-and-egg feel to it. The
results of the "uname_FOO" assignments in config.mak.uname are
consulted later in the file to _assign_ a value to SHELL_PATH on a
number of platforms. So, making the "uname_FOO" assignments themselves
depend upon SHELL_PATH is rather circular and confusing.
There was a problem hiding this comment.
On the Git mailing list, Kyohei Kadota wrote (reply to this):
> On Wed, Aug 5, 2020 at 10:14 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> > On 2020-08-06 at 01:05:01, lufia via GitGitGadget wrote:
> > > In the not POSIX environment, like Plan 9, sh might not be looked up
> > > in the directories named by the $PATH.
> >
> > I think Git's editor handling assumes that sh is somewhere in the PATH,
> > so it might be fine for us to just ask the user to adjust PATH
> > appropriately before running make. I don't have a strong preference; if
> > this works on a standard Unix machine, which it looks like it should
> > (although I haven't tested), I'm fine with it.
>
> This does, however, have a bit of a chicken-and-egg feel to it. The
> results of the "uname_FOO" assignments in config.mak.uname are
> consulted later in the file to _assign_ a value to SHELL_PATH on a
> number of platforms. So, making the "uname_FOO" assignments themselves
> depend upon SHELL_PATH is rather circular and confusing.
The problem is, Plan 9 APE's sh is placed on /bin/ape/sh by default.
By running /bin/ape/psh
APE commands existed under /bin/ape such as sh, uname, etc, will be
mounted on /bin.
In running of ape/psh, APE commands will prioritize than native commands.
So I've wanted to run /bin/ape/psh before executing some shell
commands by gmake.
----
kadota
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Eric Sunshine <sunshine@sunshineco.com> writes:
> On Wed, Aug 5, 2020 at 10:14 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
>> On 2020-08-06 at 01:05:01, lufia via GitGitGadget wrote:
>> > In the not POSIX environment, like Plan 9, sh might not be looked up
>> > in the directories named by the $PATH.
>>
>> I think Git's editor handling assumes that sh is somewhere in the PATH,
>> so it might be fine for us to just ask the user to adjust PATH
>> appropriately before running make. I don't have a strong preference; if
>> this works on a standard Unix machine, which it looks like it should
>> (although I haven't tested), I'm fine with it.
>
> This does, however, have a bit of a chicken-and-egg feel to it. The
> results of the "uname_FOO" assignments in config.mak.uname are
> consulted later in the file to _assign_ a value to SHELL_PATH on a
> number of platforms. So, making the "uname_FOO" assignments themselves
> depend upon SHELL_PATH is rather circular and confusing.
Is that just being circular and confusing, or does it produce an
incorrect result depending on the circumstances? We would end up
special casing SHELL_PATH (e.g. exclude it from the variables that
are set based on uname), which I would want to avoid, as I would
suspect that there will be even more variables that need similar
treatment.
This does have a bad smell. Even on a not-so-posix Windows, we do
not use such a hack.
There was a problem hiding this comment.
On the Git mailing list, Kyohei Kadota wrote (reply to this):
After a moment's thought I decided to run ape/psh before building git.
If so then, sh will be found from the directories named in PATH
because it is mounted onto /bin.
2020年8月7日(金) 2:30 Junio C Hamano <gitster@pobox.com>:
>
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
> > On Wed, Aug 5, 2020 at 10:14 PM brian m. carlson
> > <sandals@crustytoothpaste.net> wrote:
> >> On 2020-08-06 at 01:05:01, lufia via GitGitGadget wrote:
> >> > In the not POSIX environment, like Plan 9, sh might not be looked up
> >> > in the directories named by the $PATH.
> >>
> >> I think Git's editor handling assumes that sh is somewhere in the PATH,
> >> so it might be fine for us to just ask the user to adjust PATH
> >> appropriately before running make. I don't have a strong preference; if
> >> this works on a standard Unix machine, which it looks like it should
> >> (although I haven't tested), I'm fine with it.
> >
> > This does, however, have a bit of a chicken-and-egg feel to it. The
> > results of the "uname_FOO" assignments in config.mak.uname are
> > consulted later in the file to _assign_ a value to SHELL_PATH on a
> > number of platforms. So, making the "uname_FOO" assignments themselves
> > depend upon SHELL_PATH is rather circular and confusing.
>
> Is that just being circular and confusing, or does it produce an
> incorrect result depending on the circumstances? We would end up
> special casing SHELL_PATH (e.g. exclude it from the variables that
> are set based on uname), which I would want to avoid, as I would
> suspect that there will be even more variables that need similar
> treatment.
>
> This does have a bad smell. Even on a not-so-posix Windows, we do
> not use such a hack.
>
|
On the Git mailing list, "brian m. carlson" wrote (reply to this): |
| @@ -547,6 +547,8 @@ AR = ar | |||
| RM = rm -f | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"lufia via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: lufia <lufia@lufia.org>
See Documentation/SubmittingPatches[[real-name]].
> Plan 9's tar(1) don't support o option.
> So I changed Makefiles to replace tar commands if needed.
There is a big gap/leap between these two sentences. The first
sentence might justify TAR_XF but it does not explain why TAR_CF
is needed at all. Also there is "$(TAR) czf" that is left without
getting turned into $(TAR_CF)z.
Are there still remaining uses of $(TAR) after applying this patch?
If with a different justification, a patch that abstracts our use of
"tar" as an archiving tool into different use cases and give one
Makefile macro to each of them with more meaningful names might be
more palatable. E.g. instead of TAR_CF that abstracts only the "tar
cf" part, make "take everything in the current directory and send an
archive of it to the standard output", i.e. "tar cf - .", into a
Makefile macro, with matching "take an archive from the standard
input and extract it to the current directory", i.e. "tar xf -", as
a matching Makefile macro, perhaps---that way people without a
working "tar" might be able to use zip, cpio or pax (or a script
written around them) as an alternative "archiving tool", for
example.
Side note. The above would probably be a huge undertaking
and I am not suggesting it as a serious counterproposal.
A more plausible alternative is to just touch "$(TAR) xof -" and
replace it with $(TAR_XOF) or even $(EXTRACT_TARBALL_AS_MYSELF)
while leaving $(TAR_CF) alone. That _might_ be palatable.
I dunno.
lufia
force-pushed
the
compat-p9
branch
3 times, most recently
from
467fc80 to
6f35562
Compare
|
/submit |
|
Submitted as pull.694.v2.git.1599680861.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
| @@ -10,7 +10,7 @@ command_list () { | |||
| } | |||
There was a problem hiding this comment.
On the Git mailing list, Eric Sunshine wrote (reply to this):
On Wed, Sep 9, 2020 at 3:48 PM Kyohei Kadota via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> And its sed(1)'s label is limited to maximum seven characters.
> Therefore I replaced some labels to drop a character.
>
> * close -> cl
> * continue -> cont (cnt is used for count)
> * line -> ln
> * hered -> hdoc
> * shell -> sh
> * string -> str
These are reasonable. "cl" feels a little odd, but I can't think of
anything better.
> diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
> @@ -24,7 +24,7 @@ category_list () {
> get_synopsis () {
> sed -n '
> - /^NAME/,/'"$1"'/H
> + /^NAME/,/'"$1"'/h
This change is not mentioned in the commit message. "H" and "h" are
very different commands, so it's difficult, at a glance, to tell if
this change is even valid. Some explanation in the commit message
would help (if it is indeed valid).
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Eric Sunshine <sunshine@sunshineco.com> writes:
> On Wed, Sep 9, 2020 at 3:48 PM Kyohei Kadota via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>> And its sed(1)'s label is limited to maximum seven characters.
>> Therefore I replaced some labels to drop a character.
>>
>> * close -> cl
>> * continue -> cont (cnt is used for count)
>> * line -> ln
>> * hered -> hdoc
>> * shell -> sh
>> * string -> str
>
> These are reasonable. "cl" feels a little odd, but I can't think of
> anything better.
>
>> diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
>> @@ -24,7 +24,7 @@ category_list () {
>> get_synopsis () {
>> sed -n '
>> - /^NAME/,/'"$1"'/H
>> + /^NAME/,/'"$1"'/h
>
> This change is not mentioned in the commit message. "H" and "h" are
> very different commands, so it's difficult, at a glance, to tell if
> this change is even valid. Some explanation in the commit message
> would help (if it is indeed valid).
I am glad somebody else caught the same thing as I did. copying and
appending will give very different results.
Thanks.
There was a problem hiding this comment.
On the Git mailing list, Kyohei Kadota wrote (reply to this):
Thank you for your review!
2020年9月10日(木) 4:56 Eric Sunshine <sunshine@sunshineco.com>:
>
> On Wed, Sep 9, 2020 at 3:48 PM Kyohei Kadota via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> > And its sed(1)'s label is limited to maximum seven characters.
> > Therefore I replaced some labels to drop a character.
> >
> > * close -> cl
> > * continue -> cont (cnt is used for count)
> > * line -> ln
> > * hered -> hdoc
> > * shell -> sh
> > * string -> str
>
> These are reasonable. "cl" feels a little odd, but I can't think of
> anything better.
>
> > diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
> > @@ -24,7 +24,7 @@ category_list () {
> > get_synopsis () {
> > sed -n '
> > - /^NAME/,/'"$1"'/H
> > + /^NAME/,/'"$1"'/h
>
> This change is not mentioned in the commit message. "H" and "h" are
> very different commands, so it's difficult, at a glance, to tell if
> this change is even valid. Some explanation in the commit message
> would help (if it is indeed valid).
I missed, this change was not needed. It is remnants of trial and error.
|
User |
tr(1) of ANSI/POSIX environment, aka APE, don't support \n literal. It's handles only octal(\ooo) or hexadecimal(\xhhhh) numbers. And its sed(1)'s label is limited to maximum seven characters. Therefore I replaced some labels to drop a character. * close -> cl * continue -> cont (cnt is used for count) * line -> ln * hered -> hdoc * shell -> sh * string -> str Signed-off-by: Kyohei Kadota <lufia@lufia.org>
Plan 9's loader(aka. linker) is separated from the compiler. The compilers are called 8c, 6c... for each machine architectures; corresponded loaders are called 8l, 6l... Signed-off-by: Kyohei Kadota <lufia@lufia.org>
|
/submit |
|
Submitted as pull.694.v3.git.1599704262.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
User |
| @@ -10,7 +10,7 @@ command_list () { | |||
| } | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Kyohei Kadota via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Kyohei Kadota <lufia@lufia.org>
>
> tr(1) of ANSI/POSIX environment, aka APE, don't support \n literal.
> It's handles only octal(\ooo) or hexadecimal(\xhhhh) numbers.
>
> And its sed(1)'s label is limited to maximum seven characters.
> Therefore I replaced some labels to drop a character.
>
> * close -> cl
> * continue -> cont (cnt is used for count)
> * line -> ln
> * hered -> hdoc
> * shell -> sh
> * string -> str
>
> Signed-off-by: Kyohei Kadota <lufia@lufia.org>
> ---
This round, without the confusion between 'h' and 'H' commands, I
see nothing funny in it.
Will queue. Thanks.
| @@ -252,6 +252,7 @@ jobs: | |||
| pool: ubuntu-latest | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Kyohei Kadota via GitGitGadget" <gitgitgadget@gmail.com> writes:
> diff --git a/Makefile b/Makefile
> index 86e5411f39..c0c0c280f0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -543,6 +543,7 @@ export prefix bindir sharedir sysconfdir gitwebdir perllibdir localedir
>
> # Set our default programs
> CC = cc
> +LD = cc
This is good that any system that use 'cc' to link now will reach
the same 'cc' via $(LD) instead of $(CC).
But those who customize $(CC) to their taste now needs to override
not just CC but also LD. Can we avoid regressing for them? Perhaps
default the value of LD to $(CC) instead of hardcoded 'cc'?
> @@ -2121,7 +2122,7 @@ git.sp git.s git.o: EXTRA_CPPFLAGS = \
> '-DGIT_INFO_PATH="$(infodir_relative_SQ)"'
>
> git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
> - $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \
> + $(QUIET_LINK)$(LD) -o $@ $(ALL_LDFLAGS) \
> $(filter %.o,$^) $(LIBS)
Are there those who depended on ALL_CFLAGS on the command line when
the final linking happens?
For example, people have '-g -O2' on CFLAGS, and that will be
included in ALL_CFLAGS and passed to the final linking phase. GNU
ld has "-O" option to optimize; when existing systems link with "cc
-g -O2 -o $@ ...", it is likely that "-O2" given to "cc" will cause
the "-O" option to be passed to underlying "ld".
But with the updated procedure (where LD is 'cc' for them), the
linkage will be done without ALL_CFLAGS passed on the command line,
and results in "cc -o $@ ...", without "-g -O2".
That sounds like a regression.
> @@ -2445,17 +2446,17 @@ compat/nedmalloc/nedmalloc.sp: SP_EXTRA_FLAGS += -Wno-non-pointer-null
> endif
>
> git-%$X: %.o GIT-LDFLAGS $(GITLIBS)
> - $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
> + $(QUIET_LINK)$(LD) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
>
> git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)
> - $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> + $(QUIET_LINK)$(LD) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> $(IMAP_SEND_LDFLAGS) $(LIBS)
>
> git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)
> - $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> + $(QUIET_LINK)$(LD) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> $(CURL_LIBCURL) $(LIBS)
> git-http-push$X: http.o http-push.o GIT-LDFLAGS $(GITLIBS)
> - $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> + $(QUIET_LINK)$(LD) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> $(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)
>
> $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)
> @@ -2465,7 +2466,7 @@ $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)
> cp $< $@
>
> $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS)
> - $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> + $(QUIET_LINK)$(LD) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> $(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)
>
> @@ -2753,7 +2754,7 @@ perf: all
> t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))
>
> t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS)
> - $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)
> + $(QUIET_LINK)$(LD) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)
Likewise for all of the above.
> diff --git a/ci/lib.sh b/ci/lib.sh
> index 3eefec500d..19c5beb277 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -125,6 +125,7 @@ then
All changes to this file to pass LD=cc everywhere CC=cc is passed
look good to me.
> diff --git a/config.mak.in b/config.mak.in
> index e6a6d0f941..76ea7e781e 100644
> --- a/config.mak.in
> +++ b/config.mak.in
> @@ -2,6 +2,7 @@
> # @configure_input@
>
> CC = @CC@
> +LD = @CC@
> CFLAGS = @CFLAGS@
> CPPFLAGS = @CPPFLAGS@
> LDFLAGS = @LDFLAGS@
OK.
> diff --git a/config.mak.uname b/config.mak.uname
> index c7eba69e54..886ce068f8 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -69,6 +69,7 @@ ifeq ($(uname_S),GNU/kFreeBSD)
> endif
> ifeq ($(uname_S),UnixWare)
> CC = cc
> + LD = $(CC)
> NEEDS_SOCKET = YesPlease
> NEEDS_NSL = YesPlease
> NEEDS_SSL_WITH_CRYPTO = YesPlease
> @@ -90,6 +91,7 @@ ifeq ($(uname_S),SCO_SV)
> endif
> ifeq ($(uname_R),5)
> CC = cc
> + LD = $(CC)
> BASIC_CFLAGS += -Kthread
> endif
> NEEDS_SOCKET = YesPlease
> @@ -435,6 +437,7 @@ ifeq ($(uname_S),Windows)
> DEFAULT_HELP_FORMAT = html
>
> CC = compat/vcbuild/scripts/clink.pl
> + LD = $(CC)
> AR = compat/vcbuild/scripts/lib.pl
> CFLAGS =
> BASIC_CFLAGS = -nologo -I. -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE
> @@ -525,6 +528,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
> ifeq ($(uname_R).$(uname_V),L17.02)
> CC += -WRVU=L16.05
> endif
> + LD = $(CC)
> # Disable all optimization, seems to result in bad code, with -O or -O2
> # or even -O1 (default), /usr/local/libexec/git-core/git-pack-objects
> # abends on "git push". Needs more investigation.
Ditto.
> @@ -665,8 +669,10 @@ else
> BASIC_LDFLAGS += -Wl,--large-address-aware
> endif
> CC = gcc
> + LD = $(CC)
> COMPAT_CFLAGS += -D__USE_MINGW_ANSI_STDIO=0 -DDETECT_MSYS_TTY \
> -fstack-protector-strong
> + BASIC_LDFLAGS += -fstack-protector-strong
> EXTLIBS += -lntdll
> INSTALL = /bin/install
> NO_R_TO_GCC_LINKER = YesPlease
This is an example of the problem we discussed earlier for not
passing ALL_CFLAGS to the linking phase. I am not sure what the
best solution is, though.
Thanks.
|
I considered and decided to use different way to link objects. So this pull request is closed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've posted some commits for porting git to Plan 9.
This pull request is thing that cut off building scripts from #305 and is re-constructed that.
I expect this don't change any artifacts.
differ from v1
cc: Eric Sunshine sunshine@sunshineco.com
cc: Kyohei Kadota lufia@lufia.org