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

Remove usage of deprecated std.xml #1977

Merged
merged 1 commit into from
Oct 13, 2020
Merged

Remove usage of deprecated std.xml #1977

merged 1 commit into from
Oct 13, 2020

Conversation

andre2007
Copy link
Contributor

@andre2007 andre2007 commented Jul 20, 2020

In this case, parsing the xml with std.regex seems to make most sense.
Example XML

<metadata modelVersion="1.1.0">
  <groupId>dubpackages</groupId>
  <artifactId>maven-dubpackage</artifactId>
  <versioning>
    <latest>1.0.6</latest>
    <release>1.0.6</release>
    <versions>
      <version>1.0.5</version>
      <version>1.0.6</version>
    </versions>
    <lastUpdated>20180317184845</lastUpdated>
  </versioning>
</metadata>

@dlang-bot
Copy link
Collaborator

Thanks for your pull request and interest in making D better, @andre2007! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

@andre2007 andre2007 linked an issue Jul 20, 2020 that may be closed by this pull request
@WebFreak001
Copy link
Member

I just noticed one other thing: before it would only search for <version>...</version> inside a <versions>...</versions> block. Can it happen that in maven there is anywhere else a <version> than in a <versions> block? Like some metadata version or something.

@andre2007
Copy link
Contributor Author

You are right, I have to extend the regex to find version tags only within a versions parent tag. (Just saw an example where version tag is after artifactId).

@WebFreak001
Copy link
Member

I'm really not quite sold on the conversion to regex and manual string operations. XML could contain a lot more weird stuff like whitespace in the tags, random attributes, the tags being in a completely different nesting level (like in some sub-tags instead of the expected root tag) than expected, etc.

How big would putting the entire undeaD xml package in be?

@andre2007
Copy link
Contributor Author

3100 lines or 90kb. Should I add it?

@WebFreak001
Copy link
Member

hm I don't think I should decide on that, what does @wilzbach think?

@andre2007
Copy link
Contributor Author

@wilzbach should I add the whole undead.xml file, what do you think?

@rikkimax
Copy link
Contributor

rikkimax commented Aug 4, 2020

Alternatively do what DCD and friends do. Git submodule for script (i.e. makefile) and dub dependency for dub.

@wilzbach
Copy link
Member

wilzbach commented Aug 4, 2020

@wilzbach should I add the whole undead.xml file, what do you think?

Hmm, I unfortunately don't see a better alternative as well. This really looks like the easiest solution to this problem as long as we don't trust dub enough to bootstrap itself or want to maintain easy bootstrapping of dub on new platforms.

Though we should add a big comment on the header stating that it's dead code, link to the undead package and maybe even put it in under an undead folder as well?

Re git submodules: I don't think it matters much whether it's a git submodule or just a copy/paste as I don't expect this file to change. The git submodule will require small adaptions to the CIs, build script and dub distro packages, but that's marginal as long as it's mentioned in a changelog entry.

If no one else has a strong opinion, I would leave the final choice on copy/paste or git submodule to you. I'm fine either way.

@andre2007
Copy link
Contributor Author

As the other dependencies are also code copies and in the long run we will have a better solution, when dub bootstrapping is available, I favour copying the file into it.
I will change the pr.

@andre2007
Copy link
Contributor Author

andre2007 commented Aug 5, 2020

macOS-10.15 LDC master fails due to an unrelated Vibe-d SSL linker issue:

d_tls.a(vibe.stream.openssl.o)
"_get_rfc3526_prime_2048", referenced from:
__D4vibe6stream7openssl14OpenSSLContext11setDHParamsMFNeAyaZv in libvibe-d_tls.a(vibe.stream.openssl.o)
"_sk_num", referenced from:
__D6deimos7openssl9safestack__T10SKM_sk_numTSQBqQBm6x509v315GENERAL_NAME_stZ__TQBwZQCaFNbPSQDkQDgQDb__T8STACK_OFTQCrZQoZi in libvibe-d_tls.a(vibe.stream.openssl.o)
(maybe you meant: __D6deimos7openssl9safestack__T10SKM_sk_numTSQBqQBm6x509v315GENERAL_NAME_stZ__TQBwZQCaFNbPSQDkQDgQDb__T8STACK_OFTQCrZQoZi)
"_sk_value", referenced from:
__D6deimos7openssl9safestack__T12SKM_sk_valueTSQBsQBo6x509v315GENERAL_NAME_stZ__TQByZQCcFNbPSQDmQDiQDd__T8STACK_OFTQCrZQoiZPQDa in libvibe-d_tls.a(vibe.stream.openssl.o)
(maybe you meant: __D6deimos7openssl9safestack__T12SKM_sk_valueTSQBsQBo6x509v315GENERAL_NAME_stZ__TQByZQCcFNbPSQDmQDiQDd__T8STACK_OFTQCrZQoiZPQDa)
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
Error: /usr/bin/cc failed with status: 1
ldc2 failed with exit code 1.
[ERROR] /Users/runner/work/dub/dub/test/ddox.sh:11 command failed
[ERROR] Script failure.

@andre2007 andre2007 closed this Sep 28, 2020
@andre2007 andre2007 reopened this Sep 28, 2020
@andre2007
Copy link
Contributor Author

Ping :)

@Geod24 Geod24 merged commit 7c54f98 into dlang:master Oct 13, 2020
@andre2007 andre2007 deleted the remove-std-xml-dep branch October 16, 2020 13:09
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.

Remove the dependency on std.xml
6 participants