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.

Sign up for GitHub

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

Jump to bottom

Repository names cannot contain certain characters #4

Open
terabyte opened this issue Mar 1, 2018 · 1 comment · May be fixed by AmlingQbt/meta#10
Open

Repository names cannot contain certain characters #4

terabyte opened this issue Mar 1, 2018 · 1 comment · May be fixed by AmlingQbt/meta#10
Labels
discussion Potential change that requires discussion enhancement PENDING-REVIEW issues that already have a fix pending review/acceptance qbt impacts code in the qbt repo

Comments

@terabyte
Copy link
Member

terabyte commented Mar 1, 2018

Currently the code that parses package and repository names is extremely narrow. This was intentional. However, to better interoperate with existing code repositories, we might want to rethink this. Currently package name and tip are separated by a comma in dependency mappings in the manifest, so commas are definitely out. Many things output NAME^TIP or NAME^{TIP} so caret is also a no-go, but it appears that slashes and dashes might both be ok (dot and underscore are currently allowed).

The package and repo tips patterns are here:
https://github.com/TerabyteQbt/qbt/blob/6b09307ffd7af1eb7810d595108de34e502e512c/app/main/src/qbt/tip/AbstractTip.java#L9

It would be very very easy to simply add things to the character classes, but very very hard to ever make them tighter if we later change our minds...
@terabyte terabyte added enhancement qbt impacts code in the qbt repo discussion Potential change that requires discussion labels Mar 1, 2018
@terabyte
Copy link
Member Author

terabyte commented Mar 1, 2018 •
edited
Loading

@amling notes that the Cumulative Version code generates CVs using tab-deliminated strings, and thus if tabs were allowed in package names it could create a CV conflict. This worrisome supplemental example begs the question if there are other places we are not thinking about.
https://github.com/TerabyteQbt/qbt/blob/6b09307ffd7af1eb7810d595108de34e502e512c/app/main/src/qbt/recursive/cv/CumulativeVersion.java

terabyte added a commit that referenced this issue Mar 1, 2018
@terabyte
Transforming LocalRepoFinder and QbtRemote classes.
5459da2 
These classes allow the user to map a repo to a different name locally on disk
and on the server, allowing those places to use names which contain otherwise
invalid characters for repo names.

Resolves #5 and mitigates #4.

QbtConfig might look like this:

Function<String, String> transformNames = new Function<String, String>() {
    public String apply(String name) {
        return name.replace("flume_ng", "flume-ng").replace("hadoop_lzo", "hadoop-lzo");
    }
}

return new QbtConfig(
    new TransformingFormatLocalRepoFinder(gitLocalVcs, transformNames, workspaceRoot.toString()),
    // ...
    new MapQbtRemoteFinder([
        formatremote: new TransformingFormatQbtRemote(
            gitRemoteVcs,
            transformNames,
            "ssh://git@github.mtv.cloudera.com/TerabyteQbt/%r.git",
        ),
        githubremote: new TransformingGithubQbtRemote(
            gitRemoteVcs,
            transformNames,
            githubToken,
            "TerabyteQbt",
            "%r",
        ),
    ]),
    // ...
);
terabyte added a commit that referenced this issue Mar 1, 2018
@terabyte
Transforming LocalRepoFinder and QbtRemote classes.
6b177a8 
These classes allow the user to map a repo to a different name locally on disk
and on the server, allowing those places to use names which contain otherwise
invalid characters for repo names.

Resolves #5 and mitigates #4.

QbtConfig might look like this:

Function<String, String> transformNames = new Function<String, String>() {
    public String apply(String name) {
        return name.replace("flume_ng", "flume-ng").replace("hadoop_lzo", "hadoop-lzo");
    }
}

return new QbtConfig(
    new TransformingFormatLocalRepoFinder(gitLocalVcs, transformNames, workspaceRoot.toString()),
    // ...
    new MapQbtRemoteFinder([
        formatremote: new TransformingFormatQbtRemote(
            gitRemoteVcs,
            transformNames,
            "ssh://git@github.mtv.cloudera.com/TerabyteQbt/%r.git",
        ),
        githubremote: new TransformingGithubQbtRemote(
            gitRemoteVcs,
            transformNames,
            githubToken,
            "TerabyteQbt",
            "%r",
        ),
    ]),
    // ...
);
@terabyte terabyte mentioned this issue Mar 1, 2018
Closed
@terabyte terabyte linked a pull request Mar 16, 2018 that will close this issue
Open
@terabyte terabyte added the PENDING-REVIEW issues that already have a fix pending review/acceptance label Mar 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
discussion Potential change that requires discussion enhancement PENDING-REVIEW issues that already have a fix pending review/acceptance qbt impacts code in the qbt repo
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow packages with dashes in the name TerabyteQbt/meta
1 participant
@terabyte