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

new_http_archive should support specifying a root directory #221

Closed
shahms opened this issue May 29, 2015 · 11 comments
Closed

new_http_archive should support specifying a root directory #221

shahms opened this issue May 29, 2015 · 11 comments
Assignees
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) type: feature request

Comments

@shahms
Copy link

shahms commented May 29, 2015

At the moment, new_http_archive requires all rules in the BUILD file to be relative to the root of the archive. However, standard operating procedure for source code archives is to bundle files beneath a single directory which typically includes the project name and version (e.g. libfoo-1.2.4a.zip would unpack to libfoo-1.2.4a/). In particular, GitHub zip files follow this procedure. Being able to specify an archive root directory would make maintaining external BUILD files for these archives much simpler and seems like it should be relatively straightforward to support.

@kchodorow kchodorow self-assigned this Jun 1, 2015
@kchodorow
Copy link
Contributor

Good idea. I'll add an attribute, maybe something like root_directory.

@damienmg damienmg added type: feature request P3 We're not considering working on this, but happy to review a PR. (No assignee) labels Jun 19, 2015
@kamalmarhubi
Copy link
Contributor

Just adding a +1 here!

@kamalmarhubi
Copy link
Contributor

@kchodorow thanks!! 👍

kamalmarhubi added a commit to kamalmarhubi/sqlite3-bazel that referenced this issue Oct 1, 2015
This is adapting from a BUILD file I had that lived along side sqlite3
source, hence swaths of commented out targets. The use of SRC_PREFIX
throughout to deal with the source archive's root dir will be
unnecessary after a `root_directory` attributed is added to the
`new_http_archive` repository rule.

See bazelbuild/bazel#221
@kamalmarhubi
Copy link
Contributor

Does this actually work? I tried to use it and it doesn't seem to do the right thing.

My example: https://github.com/kamalmarhubi/sqlite3-bazel/tree/use-strip-prefix, I get errors about missing input.

The test external_integration_test also fails. It has an error in the attribute name, and when fixed I get the same problem of not finding source files:

ERROR: missing input file '@x//:w'.

Full output at: https://gist.github.com/kamalmarhubi/1128b38948a4077e376c

@kamalmarhubi
Copy link
Contributor

That's from running bazel test //src/test/shell/bazel:external_integration_test --test_output=errors after making this patch:

diff --git a/src/test/shell/bazel/external_integration_test.sh b/src/test/shell/bazel/external_integration_test.sh
index b0c709b..638d88a 100755
--- a/src/test/shell/bazel/external_integration_test.sh
+++ b/src/test/shell/bazel/external_integration_test.sh
@@ -512,7 +512,7 @@ new_http_archive(
     name = "x",
     url = "http://localhost:$nc_port/x.tar.gz",
     sha256 = "$sha256",
-    rm_path_prefix = "x/y/z",
+    strip_prefix = "x/y/z",
     build_file = "x.BUILD",
 )
 EOF

@kchodorow
Copy link
Contributor

Arg, sorry about this. I changed the name of the attribute at the 11th hour in code review and the test wasn't run by our CI because it's marked as manual. The actual fix should be in Monday, but changing these to strip_prefix should fix it: https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/bazel/repository/NewHttpArchiveFunction.java#L84-L86

@kchodorow kchodorow reopened this Oct 2, 2015
@kamalmarhubi
Copy link
Contributor

@kchodorow this was still buggy for zip archives, crashing out with NPE. fixed and submitted for review: https://bazel-review.googlesource.com/#/c/2084/

:-)

lberki pushed a commit that referenced this issue Oct 7, 2015
The removal of the prefix was incorrectly applied to the zip entry
instead of the destination.

Fixes #221

Tested:
    - Added a copy of the existing integration test with zip archive
    - bash compile.sh all

--
Change-Id: I5d5f75f66a17eb6f146afafb1f347a781490e616
Reviewed-on: https://bazel-review.googlesource.com/#/c/2084/
MOS_MIGRATED_REVID=104774296
@abergmeier
Copy link
Contributor

This does not seem to work for:

new_http_archive(
  name = "z",
  url = "http://zlib.net/zlib-1.2.8.tar.gz",
  skip_prefix = "zlib-1.2.8",
  sha256 = "2a0dd0894c35b8736ff2bee925aab35b473a6c6b432b25e56442bacb0e72bc3a",

)

I get a similar error as @kamalmarhubi

@abergmeier
Copy link
Contributor

And I ran into #520. Not sure, whether it is a variant of these problems...

@kchodorow
Copy link
Contributor

@abergmeier the option is strip_prefix, not skip_prefix (see http://bazel.io/docs/build-encyclopedia.html#new_http_archive.strip_prefix). Regardless, you should not be getting a stack trace, can you paste your output?

@HUTOYP
Copy link

HUTOYP commented Jul 28, 2017

i have a svn path, http://xxx.com/xxx/tag/xxx.1.0.1 for example.
how can i do?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) type: feature request
Projects
None yet
Development

No branches or pull requests

6 participants