Skip to content
New issue

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

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

Already on GitHub? # to your account

Apply some polish to README and Makefile, support d/l mirror in Makefile #13

Merged
merged 4 commits into from
Feb 8, 2017

Conversation

lindig
Copy link
Collaborator

@lindig lindig commented Feb 8, 2017

These two commits add a bit of polish:

  • The invocation of sources.rb is now covered by a macro in the Makefile. This makes it easier to add new name spaces (say xs-daemons) and adding a mirror for downloads.

  • Downloading from a mirror is now supported by the Makefile and explained in the help message of sources.rb.

  • Handling download mirrors in sources.rb is better explained in the help message.

This commit adds two features:

* The generated RPM SPEC files can point to a mirror for downloading
  the source code of packages

* Handling name spaces (upstream, xs) is more explicit. This is in
  anticipation of adding another one that will be handled slightly
  different because its packages will not be built by default.

Signed-off-by: Christian <christian.lindig@citrix.com>
This commit covers the invocation of sources.rb in the Makefile in a
macro to make it more uniform and easier to change. Furthermore it fixes
some typos in the documentation and expands it minimally.

Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
….99.tar.gz

This commit just changes the version to indicate that a fixed version is
used rather than HEAD of a branch.

Signed-off-by: Christian <christian.lindig@citrix.com>
Copy link
Contributor

@gaborigloi gaborigloi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I like how the duplication (the invocation of sources.rb) has been factored out, there is one possible typo though. (And other minor comments/suggestions, but these are really optional.)

Makefile Outdated

#
# build an Opam repo in build/ with all URL files pointing
# to sources in build/src.
#
repo: build
cp -r packages build
./utils/sources.rb packages/*/*/url | while read pkg url; do \
$(SRCS) | while read package url; do \
echo "http: \"file://$(SRC)/$$(basename $$url)\"" > build/packages/$$pkg/url;\
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be a typo: package vs. $$pkg?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I ran make repo, and the URLs did not get rewritten:

~/s/xs-opam> cat build/packages/url
http: "file:///home/gabori/src/xs-opam/build/src/ocaml-xenstore-clients-0.9.4.tar.gz"
~/s/xs-opam> cat build/packages/xs/xenstore_transport.0.9.4/url
archive: "https://github.com/djs55/ocaml-xenstore-clients/archive/0.9.4.tar.gz"
checksum: "470299fe940fc4b67dfeeab430c06b1d"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Will fix it.

-u, --url just emit URLs
-m, --mirror http://example.com/some/path download from mirror
-h, --help show this help
-u, --url just emit URLs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the above changes in the Makefile, this --url option isn't really needed anymore, if I'm not mistaken? But I'm fine with keeping it though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to keep it - I just gave up on using it to make things more uniform.

utils/sources.rb Outdated

Note that the mirror must end with a slash or otherwise the last
element of the path will be ignored. It is possible to provide the
--mirror flag without an argument - in which case no mirrow will be used. In the future a default mirror might be used in that case.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this line could be broken, as it is quite long.

@lindig
Copy link
Collaborator Author

lindig commented Feb 8, 2017

I've addressed the comments. I've also added one commit that renames the package for message-switch because the name did not reflect that we are using a fixed version.

Copy link
Contributor

@gaborigloi gaborigloi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now, the typo has been fixed :)
(There is one typo in the last commit message ("break long line in"), but it isn't important.)

This commit fixes an error when URLs are rewritten in the Makefile
target "repo".

Signed-off-by: Christian <christian.lindig@citrix.com>
@lindig lindig merged commit 94fb5cd into xapi-project:master Feb 8, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants