Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support for GNU configure syntax #9990

Closed
wants to merge 3 commits into from
Closed

support for GNU configure syntax #9990

wants to merge 3 commits into from

Conversation

cnd
Copy link
Contributor

@cnd cnd commented Oct 21, 2013

Re-created: #9760

Now, without adding pc-linux-gnu platform

Re-created: #9565

t will be useful for unix systems, some package managers and external tools that is passing / maybe looking for GNU configure syntax options.

One of related bugs: #5138

Also that could be useful in future if project will really install man pages / other stuff using those variables.

@brson
Copy link
Contributor

brson commented Oct 21, 2013

Thanks again @Heather 🎆

@brson
Copy link
Contributor

brson commented Oct 21, 2013

@Heather no, I'm not sure about changing unknown to pc, but I'm worried about assumptions that some of our automation might be making about the names of the triples. I'd rather just do these things separately to be safe.

@brson
Copy link
Contributor

brson commented Oct 21, 2013

Er, that last comment should have been on #9760

@cnd
Copy link
Contributor Author

cnd commented Oct 21, 2013

@brson well I can do it but I guess /usr/share/man should be $CFG_PREFIX/usr/share/man and not $CFG_PREFIX/man

@brson
Copy link
Contributor

brson commented Oct 21, 2013

@Heather Maybe $CFG_PREFIX/share/man since $CFG_PREFIX is likely to equal /usr/local, putting the man pages at /usr/local/share/man.

Here are my best guesses for all the default values:

valopt localstatedir "/var/lib" "local state directory"
valopt sysconfdir "/etc" "install system configuration files"
valopt datadir "$CFG_PREFIX/share" "install data"
valopt infodir "$CFG_PREFIX/share/info" "install additional info"
valopt mandir "$CFG_PREFIX/share/man" "install man pages in PATH"
valopt libdir "$CFG_PREFIX/lib" "install libraries" 

Presumably /var/lib and /etc don't use the prefix.

@cnd
Copy link
Contributor Author

cnd commented Oct 22, 2013

@bors it's wrong in my opinion, everything should use prefix to allow sandboxed build and installation.

@thestinger
Copy link
Contributor

DESTDIR is for setting the top-level installation directory, and is the convention used by almost every open-source project with a Makefile. The standard line in a package script is either ./configure --prefix=/usr && make DESTDIR="$pkgdir" or make PREFIX=/usr DESTDIR="$pkgdir".

@cnd
Copy link
Contributor Author

cnd commented Oct 22, 2013

@thestinger ah, right. I'm wrong there. Will use @brson solution then.

@cnd
Copy link
Contributor Author

cnd commented Oct 22, 2013

@brson added correction

@cnd
Copy link
Contributor Author

cnd commented Oct 23, 2013

@brson failed to build? where can I see exact log?

@brson
Copy link
Contributor

brson commented Oct 24, 2013

@Heather the links in bors' output link to the log files. It appears that those links disappeared when you rebased and repushed. I r+ed again so let's see what happens this time.

@cnd
Copy link
Contributor Author

cnd commented Oct 24, 2013

@brson IOError: [Errno 2] No such file or directory: 'x86_64-unknown-linux-gnu/stage0/lib/libstd-6c65cf4b443341b1-0.9-pre.so' ?

@brson
Copy link
Contributor

brson commented Oct 24, 2013

@Heather I still don't understand that error on the bots, but I did some tests locally and discovered I needed the following diff in order for all the triple variables to be set up correctly. This does the conversion from the old _TRIPLES variables to the new before the new variables are used in various calculations, reproduces some logic for assigning the target triples, and also fixes a typo.

diff --git a/configure b/configure
index 0f965fc..d8b28bb 100755
--- a/configure
+++ b/configure
@@ -616,6 +616,26 @@ do
 done
 CFG_TARGET=$V_TEMP

+# copy host-triples to target-triples so that hosts are a subset of targets
+# XXX: remove deprecated variables here
+V_TEMP=""
+for i in $CFG_HOST_TRIPLES $CFG_TARGET_TRIPLES;
+do
+   echo "$V_TEMP" | grep -qF $i || V_TEMP="$V_TEMP${V_TEMP:+ }$i"
+done
+CFG_TARGET_TRIPLES=$V_TEMP
+
+# XXX: Support for deprecated syntax, should be dropped.
+if [ ! -z "$CFG_BUILD_TRIPLE" ]; then
+    CFG_BUILD=${CFG_BUILD_TRIPLE}
+fi
+if [ ! -z "$CFG_HOST_TRIPLES" ]; then
+    CFG_HOST=${CFG_HOST_TRIPLES}
+fi
+if [ ! -z "$CFG_TARGET_TRIPLES" ]; then
+    CFG_TARGET=${CFG_TARGET_TRIPLES}
+fi
+
 # check target-specific tool-chains
 for i in $CFG_TARGET
 do
@@ -718,7 +738,7 @@ then
     CFG_LIBDIR=bin
 fi

-for h in $CFG_HOST_
+for h in $CFG_HOST
 do
     for t in $CFG_TARGET
     do
@@ -986,20 +1006,6 @@ putvar CFG_ANDROID_CROSS_PATH
 putvar CFG_MINGW32_CROSS_PATH
 putvar CFG_MANDIR

-# Support for deprecated syntax, should be dropped.
-putvar CFG_BUILD_TRIPLE
-putvar CFG_HOST_TRIPLES
-putvar CFG_TARGET_TRIPLES
-if [ ! -z "$CFG_BUILD_TRIPLE" ]; then
-    CFG_BUILD=${CFG_BUILD_TRIPLE}
-fi
-if [ ! -z "$CFG_HOST_TRIPLES" ]; then
-    CFG_HOST=${CFG_HOST_TRIPLES}
-fi
-if [ ! -z "$CFG_TARGET_TRIPLES" ]; then
-    CFG_TARGET=${CFG_TARGET_TRIPLES}
-fi
-
 if [ ! -z "$CFG_ENABLE_PAX_FLAGS" ]
 then
     putvar CFG_ENABLE_PAX_FLAGS

@cnd
Copy link
Contributor Author

cnd commented Oct 25, 2013

@brson thanks, added this patch.

@cnd
Copy link
Contributor Author

cnd commented Oct 25, 2013

  • re-based commits another time

bors added a commit that referenced this pull request Oct 25, 2013
Re-created: #9760

Now, without adding `pc-linux-gnu` platform

Re-created: #9565

t will be useful for unix systems, some package managers and external tools that is passing / maybe looking for GNU configure syntax options.

One of related bugs: #5138

Also that could be useful in future if project will really install man pages / other stuff using those variables.
@cnd
Copy link
Contributor Author

cnd commented Oct 26, 2013

@brson -m32: command not found ?

@cnd
Copy link
Contributor Author

cnd commented Oct 28, 2013

Hello... May someone help me to understand what is happening here? Should I rebase my branch now?

@brson
Copy link
Contributor

brson commented Oct 28, 2013

@Heather I'll try a build on windows and see if I can figure out what's wrong.

@brson
Copy link
Contributor

brson commented Oct 28, 2013

I can't reproduce this failure. I do see the strange errors about -m32 command not found but they don't appear to be fatal. Can you rebase?

@cnd
Copy link
Contributor Author

cnd commented Oct 29, 2013

@brson rebased

@brson
Copy link
Contributor

brson commented Oct 30, 2013

I don't want to keep asking for rebases so I rebased and reopened as #10164

@brson brson closed this Oct 30, 2013
bors added a commit that referenced this pull request Oct 31, 2013
@cnd cnd deleted the configure-changes branch November 19, 2013 04:25
flip1995 pushed a commit to flip1995/rust that referenced this pull request May 20, 2023
Add configuration options to `--explain`

This PR rearranges some modules, taking `metadata_collector` out of `internal_lints` and making public just the necessary functions for `explain()` to use.

The output looks something like this:
```sh
$ cargo run --bin cargo-clippy --manifest-path ../rust-clippy/Cargo.toml -- --explain cognitive_complexity
### What it does
Checks for methods with high cognitive complexity.

### Why is this bad?
Methods of high cognitive complexity tend to be hard to
both read and maintain. Also LLVM will tend to optimize small methods better.

### Known problems
Sometimes it's hard to find a way to reduce the
complexity.

### Example
You'll see it when you get the warning.

========================================
Configuration for clippy::cognitive_complexity:
- cognitive-complexity-threshold: The maximum cognitive complexity a function can have (default: 25)
```

Fixes rust-lang#9990
r? `@xFrednet`

---

changelog: Docs: `cargo clippy --explain LINT` now shows possible configuration options for the explained lint
[rust-lang#10751](rust-lang/rust-clippy#10751)
<!-- changelog_checked -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants