Clientless pom #8

wfscheper · 2017-06-07T20:08:14Z

Look over the API changes and let me know if you think this works for your use case. I'm still debating whether I really need to require a coordinate string, so I'd like to hear from you on that.

Remove conary-isms and use pbr.

Use six for compatibility between python2 and python3 and tox for running tests. Biggest source of actual code change is around supporting rich comparision methods in python3.

Adds the classmethods Pom.fromstring and Pom.parse. These are modeled on the lxml.etree functions of the same name. If a MavenClient is passed in, then the resulting Pom object will attempt to dynamically load data from external POM files available via the client. Sem-Ver: feature Fixes: sassoftware#4

pombredanne · 2017-06-07T21:08:55Z

@wfscheper Let me review this in details tomorrow.

pombredanne · 2017-06-07T21:14:19Z

pymaven/client.py

@@ -75,7 +75,7 @@ def __init__(self, cacheDir=None):
        if cacheDir is None:
            cacheDir = "/tmp/%s-maven-cache" % getpass.getuser()


This is POSIX specific. I think you could use tempfile.gettempdir() for portability

Good catch. Will fix this.

pombredanne · 2017-06-08T13:30:04Z

setup.cfg

+
+[bdist_wheel]
+universal = 1
+


Your would need to add license_file = LICENSE to ensure that the built wheel contains a proper license.

I think pbr handles this for me, but I'll double check that the LICENSE file is installed and fix it if its not.

it would be a pleasant surprise if pbr does this... Note also that AFAIK pbr is mostly obsolete now that the latest setuptools support extensive, declarative metadata in setup.cfg

You are correct, and I added the license_file entry to my setup.cfg.

pbr's main role is to generate the AUTHORS file and let us manage versioning through commit history.

I'm less enthused by its use of metadata in setup.cfg given that its basically a one-to-one rewrite of a setup.py. You're not really gaining much, though it is nice to have one place to put a lot of configuration. pbr also generates a changelog for you, but it's just a dump of your commit history which isn't terribly useful as a changelog.

pombredanne · 2017-06-08T13:46:40Z

@wfscheper you wrote:

I'm still debating whether I really need to require a coordinate string, so I'd like to hear from you on that.

So there are eventually two use cases:

I provide coordinates and some mechanism to fetch and resolve data (e..g. a client)
I provide a POM file

For case 2, there is no such thing as coordinates known ahead of time: these are in the POM and unknown until after parsing

So what could possibly work is this:
Update the constructor in https://github.com/wfscheper/pymaven/blob/01d41e6011e3f5281380230cb7bdbe17bab2c88e/pymaven/pom.py#L62 to:
def __init__(self, coordinate=None, client=None, pom_data=None):

Then something more or less like this:

parse pom_data if provided. Also extract the group_id, artifact_id and version from it.
if coordinates are provided... then check these are coherent with the parse (eventually dealing with properties? )

In all cases I never know coordinates ahead of time so in any subclass I would need to replicate most of the __init__ method.

pombredanne · 2017-06-08T13:57:54Z

pymaven/pom.py

            return self._pom_factory(group, artifact, version)

    @property
+    @memoize("_pom_data")
+    def pom_data(self):


I am not sure I get why you are trying to recreate some XML here... why? If I provided an actual string of XML as an input why would you recreate this to then reparse? (and as a side note this also implies that coordinates were provided)

self.pom_data is a property (a method that is used as if it were an attribute). The memoize decorator ensures that we only call self.pom_data() once, and store the result in self._pom_data.

If you used Pom.parse or Pom.fromstring to create the Pom object, then self.pom_data() is never actually called, the memoize decorator just returns the value of self._pom_data.

If you used Pom() and didn't pass a client, then I use the EMPTY_POM template and fill it out with the values from the coordinate you passed in. If you did pass in a client, then I use the client to get the POM data from the repository. Make sense?

@wfscheper It does make sense, but I cannot see any use case where you have as an input coordinates but no pom file and no client. The library is useless in this case so this should not even be allowed: you cannot do anything of value.

pombredanne · 2017-06-08T14:02:26Z

pymaven/pom.py

 from .versioning import VersionRange

-
+EMPTY_POM = """\


Rebuilding a POM as XML is likely to be rather problematic: I may have provided deps and this would ignore them. And if I want to extend this with extra attributes for #3 my hunch that this will complicate things rather badly... Why would you need this?

The intention is that I would use this when I'm provided with coordinates but no client or xml. Would it be clearer if I called it MINIMAL_POM?

The combo of coordinates with no client and no xml should be an error IMHO.

Ensure that our LICENSE file is correctly packaged and installed.

Use tempfile.mkdtemp to create the cacheDir rather than assuming we can use /tmp.

Prefer a handwritten changelog to pbr's dump of the git history.

pombredanne · 2017-06-09T10:43:56Z

I have been thinking of another possible approach....
So what about this?

Pom is created with `init(self, coordinates, client=None)
coordinates is string that is either a POM coordinates or a path to a local file. eg. something liek coordinates_or_path
2.1 if coordinates and not client, and we have POM coordinates, then init the class with just the data from the coordinates.
2.2 if coordinates and not client, and we have a path in coordinates, then init the class with loading and parsing the file at the path stored in coordinates
2.3 if coordinates and client, and we have coordinates in coordinates, then init the class with letting the client fetch the POM file for coordinates
2.4 if coordinates and client, and we have a path in coordinates, then init the class with loading and parsing the file at the path stored in coordinates

Now, I am less clear about the case of having both a path and a client....

Also, after all a client is something that can retrieve Artifacts: there could be a dumb client that knows nothing about repositories and deals only with a path as an input and can only fetch that one path (and if provided with coordinates instead, it would just initialize a Pom/Artifact with the coordinates and fetch nothing) ... This could be another way.
All food for thoughts

wfscheper · 2017-06-09T23:40:21Z

The more we discuss the Pom.__init__ signature, the more I think that the current implementation is an artifact of my original use case (which was bootstrapping the full dependency tree from just a coordinate string). That's no longer the primary use case.

I think Pom.__init__(pom_data, client=None) makes more sense for a general purpose Pom object, and we provide three classmethods that are Pom factories: Pom.parse(source, client=None), Pom.fromstring(text, client=None), and Pom.fromcoordinate(coordinate, client).
pom_data should be an xml string, and the Pom.__init__ method will handle parsing it into an ElementTree.

Sound reasonable to you?

pombredanne · 2017-06-11T14:27:39Z

Sound reasonable to you?
@wfscheper It sure does, but I am still not sure if makes sense to recreate any XML string though. It feelds weird when the input is coordinates and would be imposed if the constructor of Pom is Pom.__init__(pom_data, client=None)

wfscheper · 2017-06-11T15:13:58Z

Not completely sure what you mean by "recreate any XML string." Are you referring to where the PR was using the EMPTY_POM template? That would go away with the new signature.

On Sun, Jun 11, 2017 at 10:27 AM Philippe Ombredanne < ***@***.***> wrote: Sound reasonable to you? @wfscheper <https://github.com/wfscheper> It sure does, but I am still not sure if makes sense to recreate any XML string though. It feelds weird when the input is coordinates and would be imposed if the constructor of Pom is Pom.__init__(pom_data, client=None) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAD5s-q3n9cwb3sacMf4ka-35XqoS1baks5sC_lbgaJpZM4NzN06> .

-- Sent from my phone

pombredanne · 2017-06-11T15:31:19Z

Are you referring to where the PR was using the EMPTY_POM template? That would go away with the new signature.

perfect!

pombredanne · 2017-06-16T09:59:26Z

@wfscheper to give you a better feel of where I want to go (and what I would like to contribute back eventually for #3 #9 #10 #11 and #12 ) ... See these:

https://github.com/nexB/scancode-toolkit/blob/96f70260d8d408c2df352f65d853bcd7ec0f88fc/src/packagedcode/maven.py#L97 which contains a subclass of pom.Pom that is created from a pom file and adds:
- extra properties collection,
- a to_dict method for (recursive) serialization to builtin data structures,
- parent pom inheritance
- enhanced properties resolution with both additional properties and the handling of the rare but pesky "substring"

Note: this is based of master and not this branch, but merging will not be of issue and I will rebase as needed when times comes.

and the minor changes I made to pymaven in this branch https://github.com/sassoftware/pymaven/compare/master...nexB:additional-properties?expand=1 while we sort it all out:
- unicode
- stable ordering of deps and properties in sequences and mappings using "unique" lists rather than sets and OrderedDict rather than dict where needed
- other minor refinements to work "offline" without a "fetching" client (such as for Do not return 'None' as a property value #12 )

Note that I have over 500+ tests with several POMs with oddities that were collected mostly carefully.

pombredanne · 2017-06-21T22:12:35Z

@wfscheper any feedback on the subclass I mentioned above? I can start creating a PR, but I would rather base this on top of your refactoring than master I guess.

wfscheper · 2017-06-21T22:38:55Z

Just catching up. There's a lot here that I like, especially the tests and odd corner case poms. I'm going to merge my pom refactor into a new branch in the main repo and we can start pulling in your changes there. I'm still unsatisfied with the Pom API, and I expect we'll want to change the API some more as we work through your enhancements.

On Wed, Jun 21, 2017 at 6:12 PM Philippe Ombredanne < ***@***.***> wrote: @wfscheper <https://github.com/wfscheper> any feedback on the subclass I mentioned above? I can start creating a PR, but I would rather base this on top of your refactoring than master I guess. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAD5s66UARQ1jlCDcxwYHWXVHKp9qnniks5sGZVUgaJpZM4NzN06> .

-- Sent from my phone

pombredanne · 2017-06-24T10:24:24Z

@wfscheper this sounds like a plan! 👍

pombredanne · 2017-06-24T10:25:50Z

@wfscheper FWIW my patches have been baked in the latest release of scancode https://github.com/nexB/scancode-toolkit/releases/ .... and I would like to know if this is OK to have your name in the credits :) as this is a major improvement on our side for the Maven thingies

pombredanne · 2017-06-24T10:37:41Z

and FYI to give some context of what we are up to:

here we are collecting package info: https://github.com/nexB/scancode-toolkit/tree/develop/src/packagedcode for Maven and many (eventually) most package format. This includes direct deps and any other deeper or pinned dep (when available in some lockfile of sorts)
https://github.com/nexB/dependentcode/ will eventually use this data as an input to perform a full dep resolution (using a sat solver)

So the goals are 1. detect all the packages and all deps in a codebase. 2. report all the licensing and origin info. 3. report all known vulnerabilities for all detected or resolved packages (as part of https://github.com/nexB/vulnerablecode which is built by @kartiksibal as part of the GSoC https://summerofcode.withgoogle.com/organizations/4800434830049280/#4673431489478656 ) 4. report other useful info on this

wfscheper · 2017-06-26T16:22:32Z

Closing. We will continue work on this in the clientless-pom branch of the main repo, and merge that in once its fully baked.

wfscheper added 3 commits June 7, 2017 16:00

Change packaging to use pbr

6f97229

Remove conary-isms and use pbr.

Update for python3

fb5d351

Use six for compatibility between python2 and python3 and tox for running tests. Biggest source of actual code change is around supporting rich comparision methods in python3.

wfscheper mentioned this pull request Jun 7, 2017

Proposal: Enable testing on public CI (on Linux, macOSX and Windows) #7

Closed

pombredanne reviewed Jun 7, 2017

View reviewed changes

pombredanne reviewed Jun 8, 2017

View reviewed changes

wfscheper and others added 3 commits June 8, 2017 10:16

Add license_file to setup.cfg

fc1b58b

Ensure that our LICENSE file is correctly packaged and installed.

Make http cache directory securely

8233409

Use tempfile.mkdtemp to create the cacheDir rather than assuming we can use /tmp.

Maintain changelog manually

fd3976e

Prefer a handwritten changelog to pbr's dump of the git history.

pombredanne mentioned this pull request Jun 19, 2017

Some ideas for package manager-independent dep resolution aboutcode-org/dependency-inspector#1

Open

pombredanne mentioned this pull request Jun 24, 2017

Upstream pymaven updates and tests to pymaven aboutcode-org/scancode-toolkit#657

Open

wfscheper closed this Jun 26, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clientless pom #8

Clientless pom #8

wfscheper commented Jun 7, 2017

pombredanne commented Jun 7, 2017

pombredanne Jun 7, 2017

wfscheper Jun 8, 2017

pombredanne Jun 8, 2017

wfscheper Jun 8, 2017

pombredanne Jun 8, 2017

wfscheper Jun 8, 2017

pombredanne commented Jun 8, 2017 •

edited

Loading

pombredanne Jun 8, 2017

wfscheper Jun 8, 2017 •

edited

Loading

pombredanne Jun 9, 2017 •

edited

Loading

pombredanne Jun 8, 2017 •

edited

Loading

wfscheper Jun 8, 2017 •

edited

Loading

pombredanne Jun 9, 2017

pombredanne commented Jun 9, 2017

wfscheper commented Jun 9, 2017 •

edited

Loading

pombredanne commented Jun 11, 2017

wfscheper commented Jun 11, 2017 via email

pombredanne commented Jun 11, 2017

pombredanne commented Jun 16, 2017

pombredanne commented Jun 21, 2017

wfscheper commented Jun 21, 2017 via email

pombredanne commented Jun 24, 2017

pombredanne commented Jun 24, 2017

pombredanne commented Jun 24, 2017

wfscheper commented Jun 26, 2017

		@@ -75,7 +75,7 @@ def __init__(self, cacheDir=None):
		if cacheDir is None:
		cacheDir = "/tmp/%s-maven-cache" % getpass.getuser()


		[bdist_wheel]
		universal = 1

Clientless pom #8

Clientless pom #8

Conversation

wfscheper commented Jun 7, 2017

pombredanne commented Jun 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pombredanne commented Jun 8, 2017 • edited Loading

Choose a reason for hiding this comment

wfscheper Jun 8, 2017 • edited Loading

Choose a reason for hiding this comment

pombredanne Jun 9, 2017 • edited Loading

Choose a reason for hiding this comment

pombredanne Jun 8, 2017 • edited Loading

Choose a reason for hiding this comment

wfscheper Jun 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pombredanne commented Jun 9, 2017

wfscheper commented Jun 9, 2017 • edited Loading

pombredanne commented Jun 11, 2017

wfscheper commented Jun 11, 2017 via email

pombredanne commented Jun 11, 2017

pombredanne commented Jun 16, 2017

pombredanne commented Jun 21, 2017

wfscheper commented Jun 21, 2017 via email

pombredanne commented Jun 24, 2017

pombredanne commented Jun 24, 2017

pombredanne commented Jun 24, 2017

wfscheper commented Jun 26, 2017

pombredanne commented Jun 8, 2017 •

edited

Loading

wfscheper Jun 8, 2017 •

edited

Loading

pombredanne Jun 9, 2017 •

edited

Loading

pombredanne Jun 8, 2017 •

edited

Loading

wfscheper Jun 8, 2017 •

edited

Loading

wfscheper commented Jun 9, 2017 •

edited

Loading