Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
First try to test with modules under OSX #2
First try to test with modules under OSX #2
Changes from 2 commits
7364bc5
439596b
bcdb10c
caca0ea
10ab3be
78dde4a
972bda2
61da66f
43284cf
9f16e29
b57f7db
8907e3a
adc7292
6374ad8
130854b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Diese drei Änderungen bringen keine funktionalen Vorteile. Erst Funktion, später Kosmetik. Immerhin muß das muß das Eingang in das originale {fmt} Repository finden und dann will Victor wissen, wozu das gut ist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solange nicht alle Tests laufen finde ich das wichtig, dann kann das wieder raus.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Das sieht so aus, als ob
format.o
fehlt.Der Test hat alle Symbole doppelt: einmal im Modul
fmt
and damit attached an das Modul mit anderem Mangling, und ein zweites Mal in der Test-Infrastruktur (libtest-main
mitgtest
undfmt
) als traditioneller Build mit normalem Mangling.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Das hat nicht mit dem Module zu tun.
Bitte keine Änderungsvorschläge, die die funktionalen Tests von {fmt} betreffen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich will nur sehen, was tatsächlich wie gebaut werden soll, kann dann wieder raus.
Tatsächlich kann man so sehen, das kein test bei euch baut worden ist bisher!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: Es sind nie die test mit module kompiliert worden bisher!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Die funktionalen Tests sollen auf jeden Fall mitlaufen. Bei Compilern, die Modules beherrschen (und nur dort!) sollen zusätzlich die beiden Targets
test-module
(Erstellen des BMI und der Lib) undmodule-test
(Nutzen des BMI) erstellt und getestet werden.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Klar, bisher wurde aber mit der Module version gar kein test compiliert!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Und einfach so von der Kommandozeile? (Dateien passend zurechtgelegt)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With a little more love it works:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some progress! 🎉
The test failures are possibly related to some encoding issues. There are no functional tests for
fmt::print
with wide strings informat-test.cc
. May be you want to add them there for a cross-check.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, nun war ich ins Englische abgerutscht 🤦♀️
Ich bin nicht sicher, ob Deutsch deine Muttersprache ist. Dein Name könnte auch Schwedisch sein 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mein Name ist
Claus Klein
ausSchwäbisch Hall
(gebürtiger Pfälzer) ;-)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wenn die Test-Runner damit glücklich sind, bin ich's auch 😊
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nur solange nicht alle tests linken, und war sowie drinnen, nur ganz oben!
Ich bin im Moment nur am linken des
module tests
interessiert, solange der nicht geht.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Das und die beiden nachfolgenden Änderungen haben nicht mit dem Module zu tun! Also bitte nicht ändern, wie alle anderen bestehenden funktionalen Test aus dem Original-Repo bei Victor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
des
..
gibt eine__
im binary dir path!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Da kann ich leider nichts beitragen. Meine Grundlage ist das originale
CMakeLists.txt
mit meinen Änderungen für Modules aus 2019, welche die letzten beiden Jahre bei Victor nicht überlebt haben. CMake war unfähig, ordentliche Projekte für den GitHub CI zu erzeugen.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CMake
macht das, was man in denCMakeLists.txt
modelliert!Die Tests sind undurchsichtig, für mich nicht nachvollziehbar modelliert?
Warum wird nicht die vorgebaute
fmt
lib verwendet?Die
libfmt.a
wird 2 mal gebaut, mit gleichem Inhalt. See TODO.txt.