You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by GitBox <gi...@apache.org> on 2022/08/17 20:29:54 UTC
[GitHub] [buildstream-plugins] nanonyme opened a new pull request, #27: Have absolute path for conf-root
nanonyme opened a new pull request, #27:
URL: https://github.com/apache/buildstream-plugins/pull/27
This enables calling configure from build-root to work
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream-plugins] nanonyme closed pull request #27: Have absolute path for conf-root
Posted by GitBox <gi...@apache.org>.
nanonyme closed pull request #27: Have absolute path for conf-root
URL: https://github.com/apache/buildstream-plugins/pull/27
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream-plugins] nanonyme commented on pull request #27: Have absolute path for conf-root
Posted by GitBox <gi...@apache.org>.
nanonyme commented on PR #27:
URL: https://github.com/apache/buildstream-plugins/pull/27#issuecomment-1221400763
I am concerned whether this may leak element name (which is part of build-root) into binaries. I will iterate more on whether we can have correct reldir in this case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream-plugins] nanonyme commented on pull request #27: Have absolute path for conf-root
Posted by GitBox <gi...@apache.org>.
nanonyme commented on PR #27:
URL: https://github.com/apache/buildstream-plugins/pull/27#issuecomment-1221547272
I am having some trouble figuring out how to handle the relative path generation with all these interdependent variables. freedesktop-sdk mostly had generation as a single flat string, hence why it was easy.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream-plugins] nanonyme closed pull request #27: Have absolute path for conf-root
Posted by GitBox <gi...@apache.org>.
nanonyme closed pull request #27: Have absolute path for conf-root
URL: https://github.com/apache/buildstream-plugins/pull/27
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream-plugins] nanonyme commented on pull request #27: Have absolute path for conf-root
Posted by GitBox <gi...@apache.org>.
nanonyme commented on PR #27:
URL: https://github.com/apache/buildstream-plugins/pull/27#issuecomment-1221540676
@gtristan does the test image not have realpath?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream-plugins] nanonyme commented on a diff in pull request #27: Have absolute path for conf-root
Posted by GitBox <gi...@apache.org>.
nanonyme commented on code in PR #27:
URL: https://github.com/apache/buildstream-plugins/pull/27#discussion_r950688723
##########
src/buildstream_plugins/elements/autotools.yaml:
##########
@@ -17,6 +17,11 @@ variables:
# Project-wide extra arguments to be passed to `configure`
conf-global: ''
+ # This needs to be overridden for build-dir to work
Review Comment:
Looks like build-dir unsetting now works different than before ("." vs "" ) so I will need to spend some time fixing freedesktop-sdk. I will comment back here when I get a passing pipeline.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream-plugins] nanonyme commented on a diff in pull request #27: Have absolute path for conf-root
Posted by GitBox <gi...@apache.org>.
nanonyme commented on code in PR #27:
URL: https://github.com/apache/buildstream-plugins/pull/27#discussion_r950458548
##########
src/buildstream_plugins/elements/autotools.yaml:
##########
@@ -17,6 +17,11 @@ variables:
# Project-wide extra arguments to be passed to `configure`
conf-global: ''
+ # This needs to be overridden for build-dir to work
Review Comment:
Thanks. I will fix once I have confidence that this is sufficient to make build-dir work. I have MR open at https://gitlab.com/freedesktop-sdk/freedesktop-sdk/-/merge_requests/9438
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream-plugins] gtristan commented on pull request #27: Have absolute path for conf-root
Posted by GitBox <gi...@apache.org>.
gtristan commented on PR #27:
URL: https://github.com/apache/buildstream-plugins/pull/27#issuecomment-1221544452
> > @gtristan does the test image not have realpath?
>
> I don’t know, but I’m doubtful of relying on expansion of `realpath` in default upstream autotools build instructions
Also given we use `/bin/sh` by default I wouldn’t want to introduce bash specific `$(command …)` expansion
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream-plugins] gtristan commented on pull request #27: Have absolute path for conf-root
Posted by GitBox <gi...@apache.org>.
gtristan commented on PR #27:
URL: https://github.com/apache/buildstream-plugins/pull/27#issuecomment-1221544132
> @gtristan does the test image not have realpath?
I don’t know, but I’m doubtful of relying on expansion of `realpath` in default upstream autotools build instructions
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream-plugins] gtristan commented on a diff in pull request #27: Have absolute path for conf-root
Posted by GitBox <gi...@apache.org>.
gtristan commented on code in PR #27:
URL: https://github.com/apache/buildstream-plugins/pull/27#discussion_r949835243
##########
src/buildstream_plugins/elements/autotools.yaml:
##########
@@ -17,6 +17,11 @@ variables:
# Project-wide extra arguments to be passed to `configure`
conf-global: ''
+ # This needs to be overridden for build-dir to work
Review Comment:
Two things here:
* The sentence *"This needs to be overridden for build-dir to work command-subdir is for monorepos to work"* doesn't read very well and appears to be developer facing technical detail. As this yaml is rendered in [user facing documentation](https://apache.github.io/buildstream-plugins/elements/autotools.html), comments here should be user facing, e.g.:
* `conf-root`: The directory where the configure script will be generated and used
* `command-subdir`: The subdirectory of `%{build-root}` where commands will be run
* I think this should be positioned near the `build-dir` as they seem functionally related
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream-plugins] nanonyme commented on pull request #27: Have absolute path for conf-root
Posted by GitBox <gi...@apache.org>.
nanonyme commented on PR #27:
URL: https://github.com/apache/buildstream-plugins/pull/27#issuecomment-1221611645
Okay. I think this is a lost cause. I will revert the previous MR. I think the autotools.yml in this repo is broken by design and cannot be fixed without breaking API.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream-plugins] nanonyme commented on pull request #27: Have absolute path for conf-root
Posted by GitBox <gi...@apache.org>.
nanonyme commented on PR #27:
URL: https://github.com/apache/buildstream-plugins/pull/27#issuecomment-1221549347
Also conf-root as a concept is just *wrong*. You run configure and autoreconf from different directory. build-dir is a commonly required concept (eg glibc cannot be built without it). And autogen typically cannot be run from another directory. Many autogen scripts also forbid running them from any other directory than the directory where autogen is in. So conf-root cannot ever be changed from `.` without breaking said projects....
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream-plugins] nanonyme commented on pull request #27: Have absolute path for conf-root
Posted by GitBox <gi...@apache.org>.
nanonyme commented on PR #27:
URL: https://github.com/apache/buildstream-plugins/pull/27#issuecomment-1221548129
Especially existence of conf-root and conf-cmd seem to make configuring autotools harder, not easier.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream-plugins] nanonyme commented on a diff in pull request #27: Have absolute path for conf-root
Posted by GitBox <gi...@apache.org>.
nanonyme commented on code in PR #27:
URL: https://github.com/apache/buildstream-plugins/pull/27#discussion_r950850689
##########
src/buildstream_plugins/elements/autotools.yaml:
##########
@@ -17,6 +17,11 @@ variables:
# Project-wide extra arguments to be passed to `configure`
conf-global: ''
+ # This needs to be overridden for build-dir to work
Review Comment:
Looks like the suggested way is to leak element name into Makefile as per https://github.com/apache/buildstream-plugins/blob/master/tests/elements/autotools/elements/amhelloconfroot.bst#L14 so I'll drop trying to get relative paths done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org